fix: add inline validation message for empty subject field#560
Conversation
Closes knoxiboy#527 What was done : Audited both AskDoubt.tsx and AskAIView.tsx against the issue checklist. Most of the validation was already implemented in the codebase. The one missing piece was an inline error message for the required Subject field in the doubt form, which I added. Changes made: Added a single inline validation message below the Subject input in AskDoubt.tsx that shows "Subject is required." when the user has interacted with the field and left it empty. Uses the existing subjectWasEdited state so it doesn't show on first load. Checklist verification: * [x] Empty doubt submissions blocked, already implemented via disabled prop on submit button * [x] Empty AI prompt submissions blocked, already implemented in AskAIView.tsx * [x] Submit buttons show disabled/loading states, already implemented with Loader2 spinner * [x] Inline validation message for subject field, added in this PR * [x] Server-side validation preserved, no backend changes made
|
CodeAnt AI is reviewing your PR. |
|
@muditd27 is attempting to deploy a commit to the Karan Mani Tripathi 's projects Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
Bot
added
gssoc'26
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Technical Review
Hi @muditd27! Thank you for your contribution to DoubtDesk.
The code changes look good. Before we can complete the technical review, approve, and merge this pull request, we have one final requirement for all contributors: Please star the DoubtDesk repository.
Once you have starred the repository, please drop a comment here saying "done" (or we will automatically detect it) and we will proceed with approving and merging your PR. Thank you.
WalkthroughAdds an inline "Subject is required." validation message in AskDoubt when the edited Subject is empty, and adds an early 403 guard plus adjusted classroom-membership lookup in replies GET handling for teacher-type doubts. ChangesSubject field validation error display
Replies GET: teacher-classroom authorization
🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels: Suggested reviewers:
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 review |
1 similar comment
|
@coderabbitai review |
There was a problem hiding this comment.
Technical Review
Hi @muditd27! Thank you for your contribution to DoubtDesk.
The code changes look good. Before we can complete the technical review, approve, and merge this pull request, we have one final requirement for all contributors: Please star the DoubtDesk repository.
Once you have starred the repository, please drop a comment here saying "done" (or we will automatically detect it) and we will proceed with approving and merging your PR. Thank you.
|
CodeAnt AI finished reviewing your PR. |
|
done |
knoxiboy
removed
gssoc'26
github-actions
Bot
added
gssoc'26
|
@muditd27 look into the ci/cd failure and fix it |
|
Hi @knoxiboy, I looked into the CI failure. The unit test error is caused by a missing NEXT_PUBLIC_NEON_DB_CONNECTION_STRING environment variable in the CI environment, it's not related to my change (which only adds an inline validation message in AskDoubt.tsx). The same tests were likely failing before this PR too. 
|
|
Automated Detailed Review
Please address the above items and request a review again. |
|
CodeAnt AI is running Incremental review |
|
CodeAnt AI Incremental review completed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/AskDoubt.tsx (1)
410-423:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAssociate the inline error with the Subject input for screen readers
Line 421 adds a visual error, but the input on Line 410 is not wired with
aria-invalid/aria-describedby, so assistive tech may not announce the validation state.Suggested fix
<input + id="doubt-subject" type="text" value={subject} @@ className="w-full bg-slate-100 dark:bg-white/5 border border-slate-200 dark:border-white/10 rounded-2xl p-4 text-slate-900 dark:text-white focus:outline-none focus:border-blue-500/50 font-bold text-sm" + aria-invalid={subjectWasEdited && !subject.trim()} + aria-describedby={subjectWasEdited && !subject.trim() ? "doubt-subject-error" : undefined} required /> {subjectWasEdited && !subject.trim() && ( -<p className="text-red-400 text-xs font-semibold px-1 mt-1">Subject is required.</p> +<p id="doubt-subject-error" className="text-red-400 text-xs font-semibold px-1 mt-1">Subject is required.</p> )}As per coding guidelines, "
**/*.tsx: Review for: Accessibility (aria labels, keyboard navigation)`".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/AskDoubt.tsx` around lines 410 - 423, The Subject input in AskDoubt is showing a visual error but lacks accessibility attributes; update the input element (where value={subject} and onChange calls setSubject/setSubjectWasEdited) to include aria-invalid={subjectWasEdited && !subject.trim()} and aria-describedby pointing to the error message ID, and give the <p> error node a stable id (e.g., subject-error) that is rendered only when the error is shown so screen readers announce the validation state.Source: Coding guidelines
🧹 Nitpick comments (1)
src/components/AskDoubt.tsx (1)
422-422: ⚡ Quick winMove validation copy to constants
Line 422 introduces a hardcoded user-facing string (
"Subject is required."), which should come from a shared constant to stay consistent with the TSX guideline and future i18n/copy updates.As per coding guidelines, "
**/*.tsx: Review for: No hardcoded strings (use constants)`".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/AskDoubt.tsx` at line 422, Replace the hardcoded string "Subject is required." in the AskDoubt component with a shared validation constant: add/export a constant (e.g., SUBJECT_REQUIRED or VALIDATION_MESSAGES.SUBJECT_REQUIRED) in your shared validation/copy constants module and import it into AskDoubt.tsx, then use that constant in the <p> element (the JSX that currently renders the text with className "text-red-400 text-xs font-semibold px-1 mt-1") so the message is centralized for consistency and future i18n.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/AskDoubt.tsx`:
- Around line 410-423: The Subject input in AskDoubt is showing a visual error
but lacks accessibility attributes; update the input element (where
value={subject} and onChange calls setSubject/setSubjectWasEdited) to include
aria-invalid={subjectWasEdited && !subject.trim()} and aria-describedby pointing
to the error message ID, and give the <p> error node a stable id (e.g.,
subject-error) that is rendered only when the error is shown so screen readers
announce the validation state.
---
Nitpick comments:
In `@src/components/AskDoubt.tsx`:
- Line 422: Replace the hardcoded string "Subject is required." in the AskDoubt
component with a shared validation constant: add/export a constant (e.g.,
SUBJECT_REQUIRED or VALIDATION_MESSAGES.SUBJECT_REQUIRED) in your shared
validation/copy constants module and import it into AskDoubt.tsx, then use that
constant in the <p> element (the JSX that currently renders the text with
className "text-red-400 text-xs font-semibold px-1 mt-1") so the message is
centralized for consistency and future i18n.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b770834a-2844-4a38-9320-1e1652b5cba0
📒 Files selected for processing (1)
src/components/AskDoubt.tsx
|
Hi @knoxiboy, I looked into the failing checks. The TypeScript error is in src/app/api/replies/route.ts (line 66), a type mismatch in the SQL query, unrelated to my change in AskDoubt.tsx. The Unit Tests are now passing. The Build Check failure likely stems from the same TypeScript error. My change only adds a 3-line inline validation message in AskDoubt.tsx and doesn't touch any API routes. |
|
CodeAnt AI is running Incremental review |
|
CodeAnt AI Incremental review completed. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/app/api/replies/route.ts (1)
61-72: ⚡ Quick winAdd explicit
The
email!assertion at line 69 is safe under current flow (lines 56-57 guaranteeclassroomIdis set), but this invariant isn't obvious and could break if the code is refactored.Additionally, if we reach line 64 with a truthy
classroomId, we've already passed lines 49-55, meaning the membership was already fetched at line 50. The second query is redundant and adds an unnecessary database round-trip.♻️ Suggested improvement
A minimal fix is to add an explicit guard for
if (doubt.type === 'teacher') { if (!doubt.classroomId) { return errorResponse("Access denied", 403); } + if (!email) { + return errorResponse("Access denied", 403); + } const [membership] = await db .select() .from(membershipsTable) .where( and( - eq(membershipsTable.userEmail, email!), + eq(membershipsTable.userEmail, email), eq(membershipsTable.classroomId, doubt.classroomId) ) );For a more thorough refactor, consider hoisting the membership variable from line 50 to a wider scope so it can be reused here, avoiding the duplicate query.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/replies/route.ts` around lines 61 - 72, The code uses a non-obvious non-null assertion on email and repeats a memberships query; add an explicit guard that returns errorResponse("Access denied", 403) if email is falsy before any use, and refactor to reuse the already-fetched membership instead of running a second DB query — hoist the initial membership variable (the result of the first db.select() from membershipsTable) to a scope visible where you check doubt.classroomId and use that membership for access checks rather than calling db.select() again; ensure all references to membershipsTable and membership use the hoisted variable and remove the duplicate query.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/app/api/replies/route.ts`:
- Around line 61-72: The code uses a non-obvious non-null assertion on email and
repeats a memberships query; add an explicit guard that returns
errorResponse("Access denied", 403) if email is falsy before any use, and
refactor to reuse the already-fetched membership instead of running a second DB
query — hoist the initial membership variable (the result of the first
db.select() from membershipsTable) to a scope visible where you check
doubt.classroomId and use that membership for access checks rather than calling
db.select() again; ensure all references to membershipsTable and membership use
the hoisted variable and remove the duplicate query.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 13bb8d6c-6b74-4d89-8c7c-8a017b6e4ee5
📒 Files selected for processing (1)
src/app/api/replies/route.ts
|
Hi @knoxiboy, I've resolved the TypeScript error in replies/route.ts and added the missing framer-motion package. All checks are passing now except Vercel authorization which is a repo-level config. Ready for review. |
github-actions
Bot
added
gssoc:approved
2 participants
User description
Closes #527
What was done :
Audited both AskDoubt.tsx and AskAIView.tsx against the issue checklist. Most of the validation was already implemented in the codebase. The one missing piece was an inline error message for the required Subject field in the doubt form, which I added.
Changes made:
Added a single inline validation message below the Subject input in AskDoubt.tsx that shows "Subject is required." when the user has interacted with the field and left it empty. Uses the existing subjectWasEdited state so it doesn't show on first load.
Checklist verification:
Description
Added inline validation message below the Subject field in AskDoubt.tsx. Shows "Subject is required." when the user has interacted with the field and left it empty.
Related Issue
Closes #527
Type of Change
Screenshots (if UI change)
| Before | After |
|
|
|
How Has This Been Tested?
npm run devChecklist
npm run dev)anytypes)mainSummary by CodeRabbit
New Features
Bug Fixes
CodeAnt-AI Description
Show a required-subject message in the doubt form and prevent invalid classroom reply access
What Changed
Impact
✅ Clearer form errors✅ Fewer failed doubt submissions✅ Safer reply access for classroom doubts💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.