fix profile api#13
Conversation
📝 WalkthroughWalkthroughSchema-driven validation and error formatting were added; API routes (profile, announcements, assignments) enforce stricter parsing/validation and some new control checks (rate limit). Frontend clients were updated to handle Zod errors, adjust payload shapes, and add minor UI state/validation. tsconfig formatting/includes were updated. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/dashboard/profile/ProfileClient.tsx (1)
165-193:⚠️ Potential issue | 🟡 Minor
saveHistorysends the entireprofileas the PUT body.Spreading
...profileinto the PUT payload means every field (name, email, department, subjects, phone, bio, createdAt, _id, …) is re-submitted on every timeline edit. Extra keys likecreatedAt/_idwill now be stripped by the server'supdateSchema, but this also means:
- Any stale value in client state gets rewritten back to Mongo as a side effect of adding/editing a history entry (root cause noted in the
route.tscomment about empty-string overwrites).- If a user has the profile open in two tabs and edits history in one, it silently clobbers the other tab's unsaved fields.
Prefer sending only what changed:
Proposed fix
- body: JSON.stringify({ - ...profile, - academicHistory: history, - }), + body: JSON.stringify({ academicHistory: history }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/dashboard/profile/ProfileClient.tsx` around lines 165 - 193, The saveHistory function currently spreads the entire profile into the PUT body which can overwrite unrelated/stale fields; change the request payload to only send the updated academicHistory (and any minimal identifier if your API requires it, e.g., profile._id) instead of ...profile. Update the fetch body to JSON.stringify({ academicHistory: history }) (or JSON.stringify({ _id: profile._id, academicHistory: history }) if the server needs an id), keep the same endpoint (/api/profile) and method (PUT), and leave the response handling that calls setProfile(updated) and the existing error/toast logic intact.
♻️ Duplicate comments (1)
app/dashboard/profile/ProfileClient.tsx (1)
215-229:⚠️ Potential issue | 🟠 Major
onTimelineSubmitdrops thedescriptionfield on server round-trip.Combined with the
lib/schemas.tsissue: the form collectsdescription, you pushdata(which includes it) intocurrent, but the server'supdateSchemadoes not declaredescriptiononacademicHistoryentries and Zod strips it. AftersaveHistoryreturns,setProfile(updated)overwrites local state with the stripped version, so the description you just typed vanishes from the UI. Fix is on the schema side (see thelib/schemas.tscomment); flagging here so you can verify end-to-end after patching.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/dashboard/profile/ProfileClient.tsx` around lines 215 - 229, The profile description field is being stripped on save because the server Zod schema for academicHistory entries does not include description; update the schema used by the profile update endpoint (referenced as updateSchema in lib/schemas.ts) to include description on each academicHistory entry, ensure the server-side parsing/validation accepts and returns description, then re-run the flow from onTimelineSubmit -> saveHistory -> setProfile to verify the returned profile includes the description so the UI retains it.
🧹 Nitpick comments (1)
lib/schemas.ts (1)
3-8: Consider.strict()vs. current default strip behavior.Right now unknown keys (e.g.,
createdAt,_idthatsaveHistoryspreads fromprofile) are silently stripped, which is fine for$setbut hides client bugs. Either keep it as-is (documented), or use.strict()to reject unexpected keys and surface client-side mistakes earlier. No action required if current behavior is intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/schemas.ts` around lines 3 - 8, The current updateSchema (z.object in lib/schemas.ts) silently strips unknown keys which hides client bugs; change the schema to be strict by appending .strict() to the z.object so unexpected fields (like email, createdAt, _id from saveHistory) cause a validation error, and then update any callers (validation error handling paths/tests that call updateSchema.parse/parseAsync or safeParse) to surface or handle these errors appropriately (or, if you prefer to keep stripping, explicitly document the stripping behavior near updateSchema); reference updateSchema when making this change and adjust saveHistory callers to stop relying on silent stripping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/profile/route.ts`:
- Around line 11-17: The handler currently trusts queryUserId and only calls
auth() if the query param is missing, allowing IDOR; change the flow to call
auth() first (invoke auth() and obtain session.userId and role), then set userId
= session.userId and only allow overriding with queryUserId when the
authenticated user is an admin (e.g., session.role or session.isAdmin) or when
queryUserId === session.userId; update the authorization check around auth(),
queryUserId, and the NextResponse.json unauthorized return to enforce this
invariant before loading the Teacher document.
- Around line 66-81: Remove the dead destructure of name, department, subjects,
phone, bio, academicHistory (it’s unused; parsed.data is the source) and update
the updatePayload creation (where you build Object.fromEntries from
Object.entries(data).filter(...)) to also exclude empty-string values so fields
like phone: "" don’t unintentionally overwrite stored values; e.g. replace the
filter predicate with one that returns false for v === undefined || v === ""
(and adjust if you need to treat arrays/objects differently), keeping
updateSchema.safeParse and parsed.data as the single source of truth and
ensuring saveHistory (where the profile is later spread) won’t cause unintended
full-profile rewrites.
- Around line 23-33: The current code creates a Teacher record even when
currentUser() returns null, leading to empty name/email; update the logic in the
route handler around currentUser(), Teacher.create and the teacher assignment to
explicitly check for an authenticated clerkUser (e.g., const clerkUser = await
currentUser(); if (!clerkUser) return/throw a 401 error) before calling
Teacher.create, and only use clerkUser.fullName and clerkUser.emailAddresses
after that check so you fail fast instead of creating an invalid Teacher
document.
In `@lib/schemas.ts`:
- Around line 9-17: The academicHistory Zod schema currently defines entries
with only year and title so any user-entered description is stripped; update the
academicHistory entry schema (the z.object used inside academicHistory) to
include description: z.string().optional() so descriptions submitted by the
client (see onTimelineSubmit/openEditEntry and the AcademicHistoryEntry type)
are preserved by updateSchema.safeParse; while there, tighten year to match
/^\d{4}$/ and consider enforcing title.min(1) to mirror client-side validation.
In `@tsconfig.json`:
- Around line 36-38: The tsconfig include array contains a duplicated and
incorrect path ".next/dev/dev/types/**/*.ts"; remove that entry so only the
correct ".next/dev/types/**/*.ts" remains (i.e., delete the
".next/dev/dev/types/**/*.ts" string from the include list in tsconfig.json).
---
Outside diff comments:
In `@app/dashboard/profile/ProfileClient.tsx`:
- Around line 165-193: The saveHistory function currently spreads the entire
profile into the PUT body which can overwrite unrelated/stale fields; change the
request payload to only send the updated academicHistory (and any minimal
identifier if your API requires it, e.g., profile._id) instead of ...profile.
Update the fetch body to JSON.stringify({ academicHistory: history }) (or
JSON.stringify({ _id: profile._id, academicHistory: history }) if the server
needs an id), keep the same endpoint (/api/profile) and method (PUT), and leave
the response handling that calls setProfile(updated) and the existing
error/toast logic intact.
---
Duplicate comments:
In `@app/dashboard/profile/ProfileClient.tsx`:
- Around line 215-229: The profile description field is being stripped on save
because the server Zod schema for academicHistory entries does not include
description; update the schema used by the profile update endpoint (referenced
as updateSchema in lib/schemas.ts) to include description on each
academicHistory entry, ensure the server-side parsing/validation accepts and
returns description, then re-run the flow from onTimelineSubmit -> saveHistory
-> setProfile to verify the returned profile includes the description so the UI
retains it.
---
Nitpick comments:
In `@lib/schemas.ts`:
- Around line 3-8: The current updateSchema (z.object in lib/schemas.ts)
silently strips unknown keys which hides client bugs; change the schema to be
strict by appending .strict() to the z.object so unexpected fields (like email,
createdAt, _id from saveHistory) cause a validation error, and then update any
callers (validation error handling paths/tests that call
updateSchema.parse/parseAsync or safeParse) to surface or handle these errors
appropriately (or, if you prefer to keep stripping, explicitly document the
stripping behavior near updateSchema); reference updateSchema when making this
change and adjust saveHistory callers to stop relying on silent stripping.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6f58b95a-ed83-480e-b9f5-e9dd16cde64d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
app/api/profile/route.tsapp/dashboard/profile/ProfileClient.tsxlib/schemas.tstsconfig.json
| let userId: string | null = queryUserId; | ||
| if (!userId) { | ||
| const session = await auth() | ||
| userId = session.userId | ||
| const session = await auth(); | ||
| userId = session.userId; | ||
| } | ||
| if (!userId) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) | ||
| if (!userId) | ||
| return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); |
There was a problem hiding this comment.
GET accepts any userId from the query string with no authorization check.
queryUserId from ?userId= completely bypasses the session check — any authenticated (or even unauthenticated, since the fallback to auth() only runs when the query param is missing) caller can fetch another user's teacher profile by passing their Clerk id. Given this route returns the full Teacher document including email, that's a PII exposure / IDOR.
At minimum, require auth() first and then either (a) only allow the query override when the caller is an admin, or (b) verify queryUserId === session.userId.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/profile/route.ts` around lines 11 - 17, The handler currently trusts
queryUserId and only calls auth() if the query param is missing, allowing IDOR;
change the flow to call auth() first (invoke auth() and obtain session.userId
and role), then set userId = session.userId and only allow overriding with
queryUserId when the authenticated user is an admin (e.g., session.role or
session.isAdmin) or when queryUserId === session.userId; update the
authorization check around auth(), queryUserId, and the NextResponse.json
unauthorized return to enforce this invariant before loading the Teacher
document.
| if (!teacher) { | ||
| const clerkUser = await currentUser() | ||
| const clerkUser = await currentUser(); | ||
| const created = await Teacher.create({ | ||
| clerkId: userId, | ||
| name: clerkUser?.fullName ?? '', | ||
| email: clerkUser?.emailAddresses[0]?.emailAddress ?? '', | ||
| department: '', | ||
| name: clerkUser?.fullName ?? "", | ||
| email: clerkUser?.emailAddresses[0]?.emailAddress ?? "", | ||
| department: "", | ||
| subjects: [], | ||
| }) | ||
| teacher = created.toObject() | ||
| }); | ||
| teacher = created.toObject(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Clerk's currentUser() from @clerk/nextjs/server return null when unauthenticated, and is emailAddresses always an array on a non-null User?
💡 Result:
Yes, Clerk's currentUser from @clerk/nextjs/server returns null when the user is unauthenticated. Yes, on a non-null User object returned by currentUser, emailAddresses is always an array.
Citations:
- 1: https://clerk.com/docs/reference/nextjs/app-router/current-user
- 2: https://www.clerk.dev/docs/reference/backend/types/backend-user
- 3: https://www.clerk.com/docs/reference/backend/types/backend-user
Check for unauthenticated user before creating Teacher record.
The code silently creates an invalid Teacher with empty name and email when currentUser() returns null (which happens when the user is unauthenticated). Instead of relying on optional chaining fallbacks, add an explicit null check and fail fast:
Proposed fix
if (!teacher) {
const clerkUser = await currentUser();
+ if (!clerkUser) {
+ return NextResponse.json(
+ { error: "Unable to load Clerk user" },
+ { status: 500 },
+ );
+ }
const created = await Teacher.create({
clerkId: userId,
- name: clerkUser?.fullName ?? "",
- email: clerkUser?.emailAddresses[0]?.emailAddress ?? "",
+ name: clerkUser.fullName ?? "",
+ email: clerkUser.emailAddresses[0]?.emailAddress ?? "",
department: "",
subjects: [],
});📝 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.
| if (!teacher) { | |
| const clerkUser = await currentUser() | |
| const clerkUser = await currentUser(); | |
| const created = await Teacher.create({ | |
| clerkId: userId, | |
| name: clerkUser?.fullName ?? '', | |
| email: clerkUser?.emailAddresses[0]?.emailAddress ?? '', | |
| department: '', | |
| name: clerkUser?.fullName ?? "", | |
| email: clerkUser?.emailAddresses[0]?.emailAddress ?? "", | |
| department: "", | |
| subjects: [], | |
| }) | |
| teacher = created.toObject() | |
| }); | |
| teacher = created.toObject(); | |
| } | |
| if (!teacher) { | |
| const clerkUser = await currentUser(); | |
| if (!clerkUser) { | |
| return NextResponse.json( | |
| { error: "Unable to load Clerk user" }, | |
| { status: 500 }, | |
| ); | |
| } | |
| const created = await Teacher.create({ | |
| clerkId: userId, | |
| name: clerkUser.fullName ?? "", | |
| email: clerkUser.emailAddresses[0]?.emailAddress ?? "", | |
| department: "", | |
| subjects: [], | |
| }); | |
| teacher = created.toObject(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/profile/route.ts` around lines 23 - 33, The current code creates a
Teacher record even when currentUser() returns null, leading to empty
name/email; update the logic in the route handler around currentUser(),
Teacher.create and the teacher assignment to explicitly check for an
authenticated clerkUser (e.g., const clerkUser = await currentUser(); if
(!clerkUser) return/throw a 401 error) before calling Teacher.create, and only
use clerkUser.fullName and clerkUser.emailAddresses after that check so you fail
fast instead of creating an invalid Teacher document.
| const { name, department, subjects, phone, bio, academicHistory } = body; | ||
|
|
||
| const parsed = updateSchema.safeParse(body); | ||
|
|
||
| if (!parsed.success) { | ||
| return NextResponse.json( | ||
| { error: parsed.error.flatten() }, | ||
| { status: 400 }, | ||
| ); | ||
| } | ||
|
|
||
| const updatePayload: Record<string, unknown> = { name, subjects } | ||
| if (department !== undefined) updatePayload.department = department | ||
| if (phone !== undefined) updatePayload.phone = phone | ||
| if (bio !== undefined) updatePayload.bio = bio | ||
| if (academicHistory !== undefined) updatePayload.academicHistory = academicHistory | ||
| const data = parsed.data; | ||
|
|
||
| const updatePayload = Object.fromEntries( | ||
| Object.entries(data).filter(([_, v]) => v !== undefined), | ||
| ); |
There was a problem hiding this comment.
Dead destructure + only undefined is filtered (empty strings still overwrite).
Two things here:
- Line 66 destructures
name, department, subjects, phone, bio, academicHistoryfrombody, but none of those locals are used afterward —parsed.datais the source of truth. Leftover from the old manual-validation code; please remove. - The
Object.fromEntries(... v !== undefined)filter only dropsundefined. Because Zod applies schema defaults/strip but preserves present keys, if the client sends e.g.phone: ""(which is valid per the schema), the$setwill overwrite the stored phone with an empty string. That's arguably the point (clear a field), but combined withsaveHistoryspreading the entireprofileinto the body, it means a full-profile PUT can unintentionally re-write every field with whatever is currently in client state. Worth confirming this is the intended behavior; otherwise also filter empty strings, or have the client send only the fields it means to change.
Proposed cleanup
- const { name, department, subjects, phone, bio, academicHistory } = body;
-
const parsed = updateSchema.safeParse(body);
if (!parsed.success) {
return NextResponse.json(
{ error: parsed.error.flatten() },
{ status: 400 },
);
}
const data = parsed.data;
-
const updatePayload = Object.fromEntries(
- Object.entries(data).filter(([_, v]) => v !== undefined),
+ Object.entries(data).filter(([, v]) => v !== undefined),
);📝 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.
| const { name, department, subjects, phone, bio, academicHistory } = body; | |
| const parsed = updateSchema.safeParse(body); | |
| if (!parsed.success) { | |
| return NextResponse.json( | |
| { error: parsed.error.flatten() }, | |
| { status: 400 }, | |
| ); | |
| } | |
| const updatePayload: Record<string, unknown> = { name, subjects } | |
| if (department !== undefined) updatePayload.department = department | |
| if (phone !== undefined) updatePayload.phone = phone | |
| if (bio !== undefined) updatePayload.bio = bio | |
| if (academicHistory !== undefined) updatePayload.academicHistory = academicHistory | |
| const data = parsed.data; | |
| const updatePayload = Object.fromEntries( | |
| Object.entries(data).filter(([_, v]) => v !== undefined), | |
| ); | |
| const parsed = updateSchema.safeParse(body); | |
| if (!parsed.success) { | |
| return NextResponse.json( | |
| { error: parsed.error.flatten() }, | |
| { status: 400 }, | |
| ); | |
| } | |
| const data = parsed.data; | |
| const updatePayload = Object.fromEntries( | |
| Object.entries(data).filter(([, v]) => v !== undefined), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/profile/route.ts` around lines 66 - 81, Remove the dead destructure
of name, department, subjects, phone, bio, academicHistory (it’s unused;
parsed.data is the source) and update the updatePayload creation (where you
build Object.fromEntries from Object.entries(data).filter(...)) to also exclude
empty-string values so fields like phone: "" don’t unintentionally overwrite
stored values; e.g. replace the filter predicate with one that returns false for
v === undefined || v === "" (and adjust if you need to treat arrays/objects
differently), keeping updateSchema.safeParse and parsed.data as the single
source of truth and ensuring saveHistory (where the profile is later spread)
won’t cause unintended full-profile rewrites.
| academicHistory: z | ||
| .array( | ||
| z.object({ | ||
| year: z.string(), | ||
| title: z.string(), | ||
| }), | ||
| ) | ||
| .max(20) | ||
| .optional(), |
There was a problem hiding this comment.
academicHistory schema is missing description — user-entered descriptions will be silently stripped.
z.object() strips unknown keys by default, so any description the client sends on an academic history entry (see onTimelineSubmit/openEditEntry in ProfileClient.tsx, and the AcademicHistoryEntry type that declares description?: string) will be dropped by updateSchema.safeParse before hitting Mongo. Users will type a description, hit Save, and it disappears on next load.
Proposed fix
academicHistory: z
.array(
z.object({
year: z.string(),
title: z.string(),
+ description: z.string().optional(),
}),
)
.max(20)
.optional(),You may also want to tighten year to /^\d{4}$/ to match the client-side pattern validation, and consider .min(1) on title.
📝 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.
| academicHistory: z | |
| .array( | |
| z.object({ | |
| year: z.string(), | |
| title: z.string(), | |
| }), | |
| ) | |
| .max(20) | |
| .optional(), | |
| academicHistory: z | |
| .array( | |
| z.object({ | |
| year: z.string(), | |
| title: z.string(), | |
| description: z.string().optional(), | |
| }), | |
| ) | |
| .max(20) | |
| .optional(), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/schemas.ts` around lines 9 - 17, The academicHistory Zod schema currently
defines entries with only year and title so any user-entered description is
stripped; update the academicHistory entry schema (the z.object used inside
academicHistory) to include description: z.string().optional() so descriptions
submitted by the client (see onTimelineSubmit/openEditEntry and the
AcademicHistoryEntry type) are preserved by updateSchema.safeParse; while there,
tighten year to match /^\d{4}$/ and consider enforcing title.min(1) to mirror
client-side validation.
| ".next/dev/types/**/*.ts", | ||
| "**/*.mts" | ||
| "**/*.mts", | ||
| ".next/dev/dev/types/**/*.ts" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -HI -t d "^types$" .next 2>/dev/null || true
fd -HI -t d "^dev$" .next 2>/dev/null || trueRepository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 66
🏁 Script executed:
cat -n tsconfig.json | sed -n '30,45p'Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 406
🏁 Script executed:
git ls-files | grep -i tsconfigRepository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 97
🏁 Script executed:
rg "\.next/dev/dev" --type ts --type js --type jsonRepository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 131
🏁 Script executed:
fd -t f -name "next.config.*" -o -name ".next*" | head -20Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 314
🏁 Script executed:
rg "\.next/(types|dev)" --type ts --type js --type json --type md | head -20Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 217
🏁 Script executed:
cat next.config.js 2>/dev/null || cat next.config.ts 2>/dev/null || echo "No next.config file found"Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 799
🏁 Script executed:
rg "dev/dev" --type ts --type js --type json --type mdRepository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 131
Remove the duplicated path .next/dev/dev/types/**/*.ts from line 38 of tsconfig.json.
The path contains a duplicated dev/dev segment which doesn't match any standard Next.js type generation pattern. Line 36 already includes .next/dev/types/**/*.ts for dev environment types, making line 38 redundant and almost certainly unintended.
Proposed fix
".next/types/**/*.ts",
".next/dev/types/**/*.ts",
- "**/*.mts",
- ".next/dev/dev/types/**/*.ts"
+ "**/*.mts"
],📝 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.
| ".next/dev/types/**/*.ts", | |
| "**/*.mts" | |
| "**/*.mts", | |
| ".next/dev/dev/types/**/*.ts" | |
| ".next/dev/types/**/*.ts", | |
| "**/*.mts" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tsconfig.json` around lines 36 - 38, The tsconfig include array contains a
duplicated and incorrect path ".next/dev/dev/types/**/*.ts"; remove that entry
so only the correct ".next/dev/types/**/*.ts" remains (i.e., delete the
".next/dev/dev/types/**/*.ts" string from the include list in tsconfig.json).
|
|
Added .trim().min(1) to title and content to prevent empty or whitespace-only values |
|
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/dashboard/assignments/AssignmentsClient.tsx (2)
341-352:⚠️ Potential issue | 🟠 MajorEdit modal won't pre-populate the
deadlinefield.
assignment.deadlineis stored/returned as a full ISO datetime (e.g."2026-04-20T00:00:00.000Z"), but<input type="date">only acceptsYYYY-MM-DD. Passing the full ISO string toresetleaves the field blank on edit, and sincedeadlineisrequired, the user has to re-pick the date every time — and combined with thetoISOString()call inonSubmit(see next comment), an empty value will throw.Proposed fix
const openEdit = (a: Assignment) => { setEditing(a); reset({ title: a.title, description: a.description, subject: a.subject, class: a.class, - deadline: a.deadline, + deadline: a.deadline ? new Date(a.deadline).toISOString().slice(0, 10) : "", maxMarks: a.maxMarks, }); setModalOpen(true); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/dashboard/assignments/AssignmentsClient.tsx` around lines 341 - 352, openEdit currently passes the full ISO datetime (assignment.deadline) into reset, which leaves the <input type="date"> blank; convert the deadline to a YYYY-MM-DD string before calling reset (e.g. derive a date-only string from a.deadline using Date parsing and toISOString/slicing or a formatter) and pass that value as the deadline in the reset call inside openEdit; also guard for null/undefined a.deadline so you don't pass an invalid value to reset.
354-380:⚠️ Potential issue | 🟠 Major
new Date(data.deadline).toISOString()can throw, and failure path ignores server error message.Two issues in the submit handler:
new Date(data.deadline).toISOString()throwsRangeError: Invalid time valueifdata.deadlineis ever empty or unparseable (which is reachable today via the edit flow — see the previous comment onopenEdit). The throw happens beforefetch, outside any validation, and lands in the generic catch — the user sees "Invalid time value" as a toast.- On
!res.okthe code shows a hardcoded"Failed to save"and never readsres.json(), so the detailed Zod validation errors returned by the API (line 91‑94 ofapp/api/assignments/route.ts) are discarded. This contradicts the PR's stated goal of "always read res.json() on failure and use returned message".Proposed fix
const onSubmit = async (data: FormData) => { const url = editing ? `/api/assignments/${editing._id}` : "/api/assignments"; const method = editing ? "PUT" : "POST"; + const deadlineDate = new Date(data.deadline); + if (Number.isNaN(deadlineDate.getTime())) { + toast("Please enter a valid deadline", "error"); + return; + } try { const res = await fetch(url, { method, headers: { "Content-Type": "application/json" }, body: JSON.stringify({ ...data, - deadline: new Date(data.deadline).toISOString(), + deadline: deadlineDate.toISOString(), maxMarks: Number(data.maxMarks), }), }); if (res.ok) { toast( editing ? "Assignment updated!" : "Assignment created!", "success", ); setModalOpen(false); fetchAssignments(); - } else toast("Failed to save", "error"); + } else { + const err = await res.json().catch(() => ({})); + toast( + typeof err?.error === "string" ? err.error : "Failed to save", + "error", + ); + } } catch (error) { toast(error instanceof Error ? error.message : "Network error", "error"); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/dashboard/assignments/AssignmentsClient.tsx` around lines 354 - 380, The onSubmit handler currently converts deadline with new Date(data.deadline).toISOString() (in onSubmit) which can throw before fetch and its catch surfaces raw RangeError; fix by validating/parsing deadline before the try/fetch (e.g., check data.deadline truthy and Date.parse(data.deadline) is not NaN, then build an isoDeadline variable or set null/undefined when invalid) and use that in the request body so conversion never throws; additionally, when the response is not ok, always await res.json() and extract a useful message (e.g., response.message or formatted zod errors) to pass to toast instead of the hardcoded "Failed to save" so API validation errors returned by the assignments route are shown to the user.app/dashboard/announcements/AnnouncementsClient.tsx (2)
551-555:⚠️ Potential issue | 🟠 MajorAlign audience input with the API enum.
The placeholder suggests free-form values like
B.Tech 3rd Year, butapp/api/announcements/route.tsonly accepts"All","Students", or"Staff". Those free-form submissions will fail validation.🐛 Proposed enum-aligned control
- <input - {...register("audience")} - className="input w-full" - placeholder="All / B.Tech 3rd Year…" - /> + <select {...register("audience")} className="input w-full"> + <option value="All">All</option> + <option value="Students">Students</option> + <option value="Staff">Staff</option> + </select>Alternatively, relax the server schema if class/year-specific audiences are intended.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/dashboard/announcements/AnnouncementsClient.tsx` around lines 551 - 555, The audience input in AnnouncementsClient.tsx currently uses a free-text <input> bound via register("audience") which permits values like "B.Tech 3rd Year" but the server schema in app/api/announcements/route.ts only accepts the enum values "All", "Students", or "Staff"; replace the free-text control with an enum-aligned control (e.g., a select or radio group) that only submits one of "All" | "Students" | "Staff" (or add client-side validation/transform before form submit to map/free-text to those enums) so submissions match the server schema and avoid validation errors — update the register("audience") usage accordingly in AnnouncementsClient.tsx.
418-465:⚠️ Potential issue | 🟠 MajorStop keyboard events from action buttons too.
The toolbar stops click bubbling with
onClick, butEnter/Spaceon the Pin/Edit/Delete buttons will bubble to the parentrole="button"element'sonKeyDownhandler, unintentionally triggering the expand/collapse toggle alongside the intended action.♿ Proposed propagation fix
<div className="flex gap-1 shrink-0" onClick={(e) => e.stopPropagation()} + onKeyDown={(e) => e.stopPropagation()} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/dashboard/announcements/AnnouncementsClient.tsx` around lines 418 - 465, The action-toolbar currently prevents click bubbling via the div with className "flex gap-1 shrink-0" but not keyboard events, so Enter/Space on the Pin/Edit/Delete buttons still bubble to the parent role="button" onKeyDown and toggles expand; add an onKeyDown={(e) => e.stopPropagation()} to that same div (the container wrapping the buttons) so keyboard activations of the buttons (which call togglePin, openEdit, setDeleteTarget) do not propagate to the parent's onKeyDown while preserving the buttons' default behaviour.
🧹 Nitpick comments (3)
app/dashboard/assignments/AssignmentsClient.tsx (2)
285-290: Map is now a no-op — simplify the normalization.Since the server guarantees
kanbanStatusvia the Zod.default("todo"),raw.map((a) => ({ ...a, kanbanStatus: a.kanbanStatus }))is equivalent toraw. Either drop the map entirely or keep a defensive fallback here to match thea.kanbanStatus ?? "todo"still used on line 461.Proposed fix
- setAssignments( - raw.map((a) => ({ - ...a, - kanbanStatus: a.kanbanStatus, - })), - ); + setAssignments(raw);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/dashboard/assignments/AssignmentsClient.tsx` around lines 285 - 290, The map call that normalizes kanbanStatus is a no-op because the server Zod default guarantees kanbanStatus; update the setAssignments call in AssignmentsClient so it either assigns raw directly (setAssignments(raw)) or preserve a defensive fallback by mapping: raw.map(a => ({ ...a, kanbanStatus: a.kanbanStatus ?? "todo" })); modify the code around setAssignments and ensure consistency with the defensive usage of kanbanStatus elsewhere (e.g., where a.kanbanStatus ?? "todo" is used).
382-411:moveCardsilently succeeds on non-2xx responses.
fetchonly rejects on network errors, so a 4xx/5xx fromPUT /api/assignments/:idleaves the optimistic update in place with no rollback and no user-visible error. Given the PR's theme of surfacing server errors, worth checkingres.okand reverting (or refetching) on failure.Proposed fix
try { - await fetch(`/api/assignments/${id}`, { + const res = await fetch(`/api/assignments/${id}`, { method: "PUT", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ kanbanStatus: col, status: col === "submitted" ? "closed" : "active", }), }); + if (!res.ok) { + const err = await res.json().catch(() => ({})); + throw new Error(typeof err?.error === "string" ? err.error : `Failed with ${res.status}`); + } } catch (error) { fetchAssignments(); toast( `Failed to move assignment: ${error instanceof Error ? error.message : "Network error"}`, "error", ); console.error("moveCard error:", error); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/dashboard/assignments/AssignmentsClient.tsx` around lines 382 - 411, The moveCard optimistic update currently doesn’t handle non-2xx responses because fetch only throws on network errors; update moveCard to check the PUT response.ok after await fetch(`/api/assignments/${id}`, ...) and if not ok revert the optimistic change (e.g., call setAssignments with previous state or simply call fetchAssignments() to refresh from server), then surface an error via toast and console.error; ensure you reference the moveCard function, the PUT to `/api/assignments/${id}`, and the existing fetchAssignments() helper when implementing the rollback/refetch.app/api/assignments/route.ts (1)
28-43: Minor: validatestatusbefore any other work.The
Invalid statuscheck sits afterconnectDB()and pagination parsing, which means invalid-input requests still pay DB connection cost. Consider moving the status validation up (and using the same enum source of truth as the schema to avoid drift):- const status = searchParams.get("status"); + const ALLOWED_STATUSES = ["active", "closed"] as const; + const status = searchParams.get("status"); + if (status && !ALLOWED_STATUSES.includes(status as (typeof ALLOWED_STATUSES)[number])) { + return NextResponse.json({ error: "Invalid status" }, { status: 400 }); + }and drop the duplicate check below. Optional cleanup; not blocking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/assignments/route.ts` around lines 28 - 43, Move the status validation to the top of the handler so invalid requests are rejected before doing extra work (e.g., before calling connectDB() and parsing pagination); specifically validate the incoming status (the status variable from searchParams.get) against the canonical enum used by your schema (instead of the inline ["active","closed"]) and return NextResponse.json({ error: "Invalid status" }, { status: 400 }) immediately on failure, then remove the duplicate check later in the function; reference the status variable, searchParams.get("status"), and NextResponse.json when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/announcements/route.ts`:
- Around line 67-73: The rate-limit check uses recentCount > 10 which allows an
11th announcement; update the condition in the request handler where recentCount
is computed (the Announcement.countDocuments call and the subsequent if) to use
>= 10 so that creating the 11th item is blocked when the intended cap is 10 per
minute, and consider replacing the non-atomic countDocuments + create flow with
an atomic counter/token-bucket later if strict enforcement is required.
- Around line 84-89: The announcement creation uses audience: data.audience ??
"all" which mismatches the validation/model allowed values ("All" | "Students" |
"Staff") and the model default "All"; update the create call in
Announcement.create to either omit the audience property when data.audience is
undefined or set it to the correctly-cased default "All" (e.g., use
data.audience ?? "All") so stored records match the schema and model defaults;
locate this change around the Announcement.create invocation to adjust the
audience assignment.
- Around line 8-9: The Zod schema in app/api/announcements/route.ts currently
allows unlimited lengths for title and content; update the announcement
validation schema (the Zod object that defines title and content) to enforce
server-side maximum lengths by changing title: z.string().trim().min(1) to
z.string().trim().min(1).max(<TITLE_MAX>) and content: z.string().trim().min(1)
to z.string().trim().min(1).max(<CONTENT_MAX>), replacing <TITLE_MAX> and
<CONTENT_MAX> with the confirmed product limits before merging so the API
rejects overly large payloads.
In `@app/api/assignments/route.ts`:
- Around line 89-94: The code uses the deprecated instance method
parsed.error.flatten(); update the error flattening to the Zod 4 top-level
function by replacing parsed.error.flatten() with z.flattenError(parsed.error).
Locate the AssignmentSchema.safeParse(...) usage and the subsequent
NextResponse.json(...) call and pass z.flattenError(parsed.error) as the error
payload so the response continues to return the same formErrors and fieldErrors
structure.
In `@lib/utils.ts`:
- Around line 1-17: formatZodError currently assumes a Zod-like shape and
returns [] for strings/null/other errors; update formatZodError to first guard
against null/undefined, then detect Zod-style shapes (formErrors/fieldErrors)
and build messages as now, but if those keys are absent fall back to returning a
single-element array containing a safe string representation of the error
(prefer error.message when present, else String(error)); ensure all property
accesses (error.formErrors, error.fieldErrors) are safe to avoid throws.
---
Outside diff comments:
In `@app/dashboard/announcements/AnnouncementsClient.tsx`:
- Around line 551-555: The audience input in AnnouncementsClient.tsx currently
uses a free-text <input> bound via register("audience") which permits values
like "B.Tech 3rd Year" but the server schema in app/api/announcements/route.ts
only accepts the enum values "All", "Students", or "Staff"; replace the
free-text control with an enum-aligned control (e.g., a select or radio group)
that only submits one of "All" | "Students" | "Staff" (or add client-side
validation/transform before form submit to map/free-text to those enums) so
submissions match the server schema and avoid validation errors — update the
register("audience") usage accordingly in AnnouncementsClient.tsx.
- Around line 418-465: The action-toolbar currently prevents click bubbling via
the div with className "flex gap-1 shrink-0" but not keyboard events, so
Enter/Space on the Pin/Edit/Delete buttons still bubble to the parent
role="button" onKeyDown and toggles expand; add an onKeyDown={(e) =>
e.stopPropagation()} to that same div (the container wrapping the buttons) so
keyboard activations of the buttons (which call togglePin, openEdit,
setDeleteTarget) do not propagate to the parent's onKeyDown while preserving the
buttons' default behaviour.
In `@app/dashboard/assignments/AssignmentsClient.tsx`:
- Around line 341-352: openEdit currently passes the full ISO datetime
(assignment.deadline) into reset, which leaves the <input type="date"> blank;
convert the deadline to a YYYY-MM-DD string before calling reset (e.g. derive a
date-only string from a.deadline using Date parsing and toISOString/slicing or a
formatter) and pass that value as the deadline in the reset call inside
openEdit; also guard for null/undefined a.deadline so you don't pass an invalid
value to reset.
- Around line 354-380: The onSubmit handler currently converts deadline with new
Date(data.deadline).toISOString() (in onSubmit) which can throw before fetch and
its catch surfaces raw RangeError; fix by validating/parsing deadline before the
try/fetch (e.g., check data.deadline truthy and Date.parse(data.deadline) is not
NaN, then build an isoDeadline variable or set null/undefined when invalid) and
use that in the request body so conversion never throws; additionally, when the
response is not ok, always await res.json() and extract a useful message (e.g.,
response.message or formatted zod errors) to pass to toast instead of the
hardcoded "Failed to save" so API validation errors returned by the assignments
route are shown to the user.
---
Nitpick comments:
In `@app/api/assignments/route.ts`:
- Around line 28-43: Move the status validation to the top of the handler so
invalid requests are rejected before doing extra work (e.g., before calling
connectDB() and parsing pagination); specifically validate the incoming status
(the status variable from searchParams.get) against the canonical enum used by
your schema (instead of the inline ["active","closed"]) and return
NextResponse.json({ error: "Invalid status" }, { status: 400 }) immediately on
failure, then remove the duplicate check later in the function; reference the
status variable, searchParams.get("status"), and NextResponse.json when making
this change.
In `@app/dashboard/assignments/AssignmentsClient.tsx`:
- Around line 285-290: The map call that normalizes kanbanStatus is a no-op
because the server Zod default guarantees kanbanStatus; update the
setAssignments call in AssignmentsClient so it either assigns raw directly
(setAssignments(raw)) or preserve a defensive fallback by mapping: raw.map(a =>
({ ...a, kanbanStatus: a.kanbanStatus ?? "todo" })); modify the code around
setAssignments and ensure consistency with the defensive usage of kanbanStatus
elsewhere (e.g., where a.kanbanStatus ?? "todo" is used).
- Around line 382-411: The moveCard optimistic update currently doesn’t handle
non-2xx responses because fetch only throws on network errors; update moveCard
to check the PUT response.ok after await fetch(`/api/assignments/${id}`, ...)
and if not ok revert the optimistic change (e.g., call setAssignments with
previous state or simply call fetchAssignments() to refresh from server), then
surface an error via toast and console.error; ensure you reference the moveCard
function, the PUT to `/api/assignments/${id}`, and the existing
fetchAssignments() helper when implementing the rollback/refetch.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4eaf44b1-6190-4fb4-9e7e-ece76657dfdd
📒 Files selected for processing (6)
app/api/announcements/route.tsapp/api/assignments/route.tsapp/dashboard/announcements/AnnouncementsClient.tsxapp/dashboard/assignments/AssignmentsClient.tsxapp/dashboard/profile/ProfileClient.tsxlib/utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/dashboard/profile/ProfileClient.tsx
| title: z.string().trim().min(1), | ||
| content: z.string().trim().min(1), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n app/api/announcements/route.ts | head -50Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 1935
🏁 Script executed:
find . -type f -name "*.ts" -o -name "*.tsx" -o -name "*.js" | xargs grep -l "AnnouncementSchema\|announcement.*schema" 2>/dev/null | head -10Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 141
🏁 Script executed:
rg "max\(" app/api/announcements/ -A 2 -B 2Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 66
🏁 Script executed:
rg "title.*max|content.*max" --type ts --type tsx -iRepository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 111
🏁 Script executed:
find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.md" \) -exec grep -l "announcement" {} \; 2>/dev/null | head -15Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 290
🏁 Script executed:
cat -n app/api/announcements/route.ts | tail -n +50 | head -80Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 1900
🏁 Script executed:
cat -n models/Announcement.tsRepository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 1139
🏁 Script executed:
cat -n app/dashboard/announcements/AnnouncementsClient.tsx | head -150Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 5326
🏁 Script executed:
rg "maxLength|max.*content|max.*title" --type ts -iRepository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 159
🏁 Script executed:
cat -n app/dashboard/announcements/AnnouncementsClient.tsx | tail -n +150 | head -200Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 8070
🏁 Script executed:
rg "register.*title|register.*content" app/dashboard/announcements/ -A 3 -B 1Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 991
🏁 Script executed:
grep -n "maxLength\|placeholder" app/dashboard/announcements/AnnouncementsClient.tsxRepository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 248
Add backend length constraints for title and content fields.
The schema currently accepts min(1) with no maximum, allowing arbitrarily large payloads. Client validation can be bypassed by directly calling the API, so server-side limits are essential.
🛡️ Proposed schema update
- title: z.string().trim().min(1),
- content: z.string().trim().min(1),
+ title: z.string().trim().min(1).max(120),
+ content: z.string().trim().min(1).max(5000),Confirm the exact product limits for title and content length before merging.
📝 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.
| title: z.string().trim().min(1), | |
| content: z.string().trim().min(1), | |
| title: z.string().trim().min(1).max(120), | |
| content: z.string().trim().min(1).max(5000), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/announcements/route.ts` around lines 8 - 9, The Zod schema in
app/api/announcements/route.ts currently allows unlimited lengths for title and
content; update the announcement validation schema (the Zod object that defines
title and content) to enforce server-side maximum lengths by changing title:
z.string().trim().min(1) to z.string().trim().min(1).max(<TITLE_MAX>) and
content: z.string().trim().min(1) to
z.string().trim().min(1).max(<CONTENT_MAX>), replacing <TITLE_MAX> and
<CONTENT_MAX> with the confirmed product limits before merging so the API
rejects overly large payloads.
| const recentCount = await Announcement.countDocuments({ | ||
| teacherId: userId, | ||
| createdAt: { $gte: new Date(Date.now() - 60 * 1000) }, | ||
| }); | ||
|
|
||
| if (recentCount > 10) { | ||
| return NextResponse.json({ error: "Too many requests" }, { status: 429 }); |
There was a problem hiding this comment.
Fix the rate-limit off-by-one.
With recentCount > 10, the 11th announcement in a minute is still allowed. Use >= 10 if the intended cap is 10 per minute.
This remains a soft throttle because countDocuments + create is non-atomic; use an atomic counter/token bucket if strict enforcement matters.
🐛 Proposed off-by-one fix
- if (recentCount > 10) {
+ if (recentCount >= 10) {
return NextResponse.json({ error: "Too many requests" }, { status: 429 });
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/announcements/route.ts` around lines 67 - 73, The rate-limit check
uses recentCount > 10 which allows an 11th announcement; update the condition in
the request handler where recentCount is computed (the
Announcement.countDocuments call and the subsequent if) to use >= 10 so that
creating the 11th item is blocked when the intended cap is 10 per minute, and
consider replacing the non-atomic countDocuments + create flow with an atomic
counter/token-bucket later if strict enforcement is required.
| const announcement = await Announcement.create({ | ||
| title: data.title, | ||
| content: data.content, | ||
| audience: data.audience ?? "all", | ||
| category: data.category ?? "general", | ||
| pinned: data.pinned ?? false, |
There was a problem hiding this comment.
Keep the default audience consistent with validation.
Line 87 stores "all", but the schema accepts "All" | "Students" | "Staff" and the model default is "All". Omitted audience values will be saved in a form the API itself rejects later.
🐛 Proposed casing fix
- audience: data.audience ?? "all",
+ audience: data.audience ?? "All",📝 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.
| const announcement = await Announcement.create({ | |
| title: data.title, | |
| content: data.content, | |
| audience: data.audience ?? "all", | |
| category: data.category ?? "general", | |
| pinned: data.pinned ?? false, | |
| const announcement = await Announcement.create({ | |
| title: data.title, | |
| content: data.content, | |
| audience: data.audience ?? "All", | |
| category: data.category ?? "general", | |
| pinned: data.pinned ?? false, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/announcements/route.ts` around lines 84 - 89, The announcement
creation uses audience: data.audience ?? "all" which mismatches the
validation/model allowed values ("All" | "Students" | "Staff") and the model
default "All"; update the create call in Announcement.create to either omit the
audience property when data.audience is undefined or set it to the
correctly-cased default "All" (e.g., use data.audience ?? "All") so stored
records match the schema and model defaults; locate this change around the
Announcement.create invocation to adjust the audience assignment.
| const parsed = AssignmentSchema.safeParse(body); | ||
| if (!parsed.success) | ||
| return NextResponse.json( | ||
| { error: parsed.error.flatten() }, | ||
| { status: 400 }, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether a shared Zod error formatter exists in the repo and how the other routes surface validation errors.
fd -t f 'formatZodError' -x cat {}
rg -nP '\b(flatten|flattenError|treeifyError|formatZodError)\b' --type=ts -C2Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 3506
🏁 Script executed:
cat -n lib/utils.ts | head -20Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 605
🏁 Script executed:
# Check what Zod version is being used
cat package.json | grep -A2 -B2 '"zod"'Repository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 191
🌐 Web query:
Zod 4 migration guide .flatten() deprecated z.flattenError
💡 Result:
In Zod 4, the .flatten method on ZodError has been deprecated. Replace it with the top-level z.flattenError(error) function. z.flattenError returns an object with: - formErrors: array of top-level errors - fieldErrors: object mapping field names to arrays of error messages Example: const flattened = z.flattenError(result.error); console.log(flattened.fieldErrors.username); // ['Expected string, received number'] Note: There is no z.flattenError type or function mentioned in the docs. The replacement is z.flattenError, which produces a flattened structure suitable for form handling. For nested schemas, use z.treeifyError instead. The official migration guide confirms: ".flatten The .flatten method on ZodError has also been deprecated. Instead use the top-level z.treeifyError function. Read the Formatting errors docs for more information." But the formatting docs clarify z.flattenError as the direct functional equivalent for flat errors.
Citations:
- 1: https://zod.dev/v4/changelog
- 2: https://www.zod.dev/v4/changelog?id=deprecates-flatten
- 3: https://v4.zod.dev/v4/changelog?id=deprecates-flatten
- 4: https://www.mintlify.com/colinhacks/zod/migration/v4-changelog
- 5: https://v4.zod.dev/error-formatting?id=zflattenerror
- 6: https://zod.dev/ERROR_HANDLING?id=flattening-errors
- 7: https://zod.dev/error-formatting
- 8: https://zod.dev/error-formatting?id=zflattenerror
🏁 Script executed:
head -30 app/api/assignments/route.tsRepository: JavaScript-Mastery-Pro/discord-challenge
Length of output: 1154
Replace deprecated parsed.error.flatten() with z.flattenError(parsed.error).
In Zod 4, the .flatten() instance method on ZodError is deprecated. Use the top-level z.flattenError() function instead—both produce identical output structure with formErrors and fieldErrors properties.
Proposed fix
const parsed = AssignmentSchema.safeParse(body);
if (!parsed.success)
return NextResponse.json(
- { error: parsed.error.flatten() },
+ { error: z.flattenError(parsed.error) },
{ status: 400 },
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/assignments/route.ts` around lines 89 - 94, The code uses the
deprecated instance method parsed.error.flatten(); update the error flattening
to the Zod 4 top-level function by replacing parsed.error.flatten() with
z.flattenError(parsed.error). Locate the AssignmentSchema.safeParse(...) usage
and the subsequent NextResponse.json(...) call and pass
z.flattenError(parsed.error) as the error payload so the response continues to
return the same formErrors and fieldErrors structure.
| export function formatZodError(error: any) { | ||
| const messages: string[] = []; | ||
|
|
||
| if (error.formErrors?.length) { | ||
| messages.push(...error.formErrors); | ||
| } | ||
|
|
||
| if (error.fieldErrors) { | ||
| for (const key in error.fieldErrors) { | ||
| const errs = error.fieldErrors[key]; | ||
| if (errs) { | ||
| messages.push(...errs.map((e: string) => `${key}: ${e}`)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return messages; |
There was a problem hiding this comment.
Handle non-validation error shapes too.
formatZodError("Too many requests") returns [], so callers show no toast for rate-limit/internal errors. null/undefined can also throw on error.formErrors.
🛡️ Proposed fallback-safe formatter
-export function formatZodError(error: any) {
+type FlattenedZodError = {
+ formErrors?: string[];
+ fieldErrors?: Record<string, string[] | undefined>;
+};
+
+export function formatZodError(error: unknown): string[] {
+ if (typeof error === "string") return [error];
+ if (!error || typeof error !== "object") return ["Request failed"];
+
+ const { formErrors, fieldErrors } = error as FlattenedZodError;
const messages: string[] = [];
- if (error.formErrors?.length) {
- messages.push(...error.formErrors);
+ if (Array.isArray(formErrors) && formErrors.length) {
+ messages.push(...formErrors);
}
- if (error.fieldErrors) {
- for (const key in error.fieldErrors) {
- const errs = error.fieldErrors[key];
- if (errs) {
+ if (fieldErrors && typeof fieldErrors === "object") {
+ for (const [key, errs] of Object.entries(fieldErrors)) {
+ if (Array.isArray(errs)) {
messages.push(...errs.map((e: string) => `${key}: ${e}`));
}
}
}
- return messages;
+ return messages.length ? messages : ["Request failed"];
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/utils.ts` around lines 1 - 17, formatZodError currently assumes a
Zod-like shape and returns [] for strings/null/other errors; update
formatZodError to first guard against null/undefined, then detect Zod-style
shapes (formErrors/fieldErrors) and build messages as now, but if those keys are
absent fall back to returning a single-element array containing a safe string
representation of the error (prefer error.message when present, else
String(error)); ensure all property accesses (error.formErrors,
error.fieldErrors) are safe to avoid throws.
1 participant
Summary by CodeRabbit