Skip to content

Fix/bughunt konstanty #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Tiin0 wants to merge 3 commits into
base: main
Choose a base branch
from
+30 −21
Open

Fix/bughunt konstanty #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4bfe6cb
Fixed error message handling in api/grade/route.ts
Tiin0 Apr 19, 2026
8c1b1d8
Fixed average score by Subject function
Tiin0 Apr 19, 2026
12b0bc4
Fixed Profile API request bug
Tiin0 Apr 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions app/api/grades/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export async function GET(req: NextRequest) {
return NextResponse.json(grades)
} catch (error) {
console.error('GET /api/grades error:', error instanceof Error ? error.message : error)
return NextResponse.json({ error: error instanceof Error ? error.stack : 'Internal server error' }, { status: 500 })
return NextResponse.json({ error:'Internal server error' }, { status: 500 })
}
}

Expand All @@ -67,13 +67,13 @@ export async function POST(req: NextRequest) {
}

const parsed = GradeSchema.safeParse(body)
if (!parsed.success) return NextResponse.json({ error: parsed.error.flatten() }, { status: 400 })
if (!parsed.success) return NextResponse.json({ error: parsed.error }, { status: 400 })
Comment on lines 69 to +70
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

Zod ZodError toJSON vs flatten() vs format() recommended error response shape

💡 Result:

For ZodError error handling in Zod (stable v4 as of 2026), use these methods to shape responses: - error.toJSON: Serializes the ZodError to JSON, producing {name: 'ZodError', issues: [...]}, where issues is the raw array of ZodIssue objects (code, path, message, etc.). Suitable for simple API responses but retains full structured details. Note: JSON.stringify(error) uses this via the Error prototype's message getter, which stringifies issues. - error.flatten or z.flattenError(error): Recommended for flat schemas (most common, e.g., forms, top-level API objects). Returns {formErrors: string[], fieldErrors: {[key: string]: string[]}}, grouping top-level errors and per-field error arrays. Ideal for client-side form error display or simple API field-error responses. - error.format or z.formatError(error): Deprecated; use z.treeifyError(error) instead. treeifyError produces a nested object mirroring the schema structure: {errors: string[], properties: {...nested errors...}}. Best for deeply nested schemas. Recommended error response shape depends on use case: - For API responses (e.g., HTTP 400), use flattened shape for simplicity: {formErrors: [...], fieldErrors: {field1: [...], ...}}. Easy to parse, matches common patterns (React Hook Form, etc.), avoids deep nesting. - Raw error.toJSON if full issue details needed (codes, expected types). - treeifyError only for complex nested data. Official docs emphasize flatten for flat schemas (majority) and treeify for nested. Examples show flatten for form handling. GitHub issue confirms JSON serialization via issues array, with caveats for client reconstruction (use new ZodError(issues)). Examples: const flattened = error.flatten; // {formErrors: ['...'], fieldErrors: {username: ['...']}} res.status(400).json(flattened); // API response JSON.stringify(error); // '{"name":"ZodError","issues":[...]}'

Citations:

🏁 Script executed:

cat -n app/api/grades/route.ts | head -80 | tail -20

Repository: JavaScript-Mastery-Pro/discord-challenge

Length of output: 903

🏁 Script executed:

git log --oneline -5 -- app/api/grades/route.ts

Repository: JavaScript-Mastery-Pro/discord-challenge

Length of output: 121

🏁 Script executed:

rg "NextResponse.json.*error" app/api/grades/route.ts -A 1

Repository: JavaScript-Mastery-Pro/discord-challenge

Length of output: 629

🏁 Script executed:

rg "GradeSchema" app/api/grades/route.ts -B 5 -A 10

Repository: JavaScript-Mastery-Pro/discord-challenge

Length of output: 1257

🏁 Script executed:

git diff HEAD~ app/api/grades/route.ts 2>/dev/null || echo "No previous version"

Repository: JavaScript-Mastery-Pro/discord-challenge

Length of output: 103

🏁 Script executed:

fd "GradeSchema" --type f --exec grep -l "GradeSchema" {} \;

Repository: JavaScript-Mastery-Pro/discord-challenge

Length of output: 66

Validation error response uses raw ZodError instead of flattened shape.

Line 70 returns parsed.error directly, which serializes to {name: 'ZodError', issues: [...]} and exposes Zod internals. This breaks consistency: every other error in this file uses a plain string ({ error: 'string' }), and this ZodError shape will confuse clients expecting a uniform error contract.

Since GradeSchema is flat (no nested objects), use .flatten() to produce {formErrors: string[], fieldErrors: {field: string[]}}, which is both client-friendly and the documented Zod best practice for flat schemas.

♻️ Suggested fix 
-    if (!parsed.success) return NextResponse.json({ error: parsed.error }, { status: 400 })
+    if (!parsed.success) return NextResponse.json({ error: parsed.error.flatten() }, { status: 400 })
📝 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.

Suggested change
const parsed = GradeSchema.safeParse(body)
if (!parsed.success) return NextResponse.json({ error: parsed.error.flatten() }, { status: 400 })
if (!parsed.success) return NextResponse.json({ error: parsed.error }, { status: 400 })
const parsed = GradeSchema.safeParse(body)
if (!parsed.success) return NextResponse.json({ error: parsed.error.flatten() }, { status: 400 })
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/api/grades/route.ts` around lines 69 - 70, The validation response
currently returns the raw ZodError (parsed.error) from the GradeSchema
safeParse; change the handler to return the flattened error shape instead by
calling parsed.error.flatten() and pass that flattened result into
NextResponse.json so the response uses formErrors/fieldErrors rather than
exposing Zod internals (update the code around the GradeSchema safeParse and the
NextResponse.json error return).


const data = parsed.data
const max = data.maxMarks!
const term = data.term ?? 'Term 1'
const grade = Grade.findOneAndUpdate(
const term = data.term ?? 'Term 1'

const grade = await Grade.findOneAndUpdate(
{ teacherId: userId, studentId: data.studentId, subject: data.subject, term },
{ $set: { ...data, term, teacherId: userId, grade: calcGrade(data.marks, max) } },
{ upsert: true, new: true }
Expand All @@ -83,6 +83,6 @@ export async function POST(req: NextRequest) {
if (error instanceof Error) {
console.error('POST /api/grades error:', error.message)
}
return NextResponse.json({ error: error instanceof Error ? error.stack : 'Internal server error' }, { status: 500 })
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
}
}
26 changes: 17 additions & 9 deletions app/api/profile/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,32 +7,40 @@ export async function GET(req: NextRequest) {
const { searchParams } = new URL(req.url)
const queryUserId = searchParams.get('userId')

let userId: string | null = queryUserId
// 1. Identificazione utente
const session = await auth()
const userId = queryUserId || session?.userId

if (!userId) {
const session = await auth()
userId = session.userId
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
Comment on lines +10 to +15
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Do not let queryUserId bypass authentication or override the session user.

Line 12 trusts queryUserId before confirming an authenticated session. This allows /api/profile?userId=... to read another profile unauthenticated if it exists, and can also create mismatched records later by combining a supplied clerkId with the current Clerk user data.

🔒 Proposed fix 
-  const session = await auth()
-  const userId = queryUserId || session?.userId
+  const { userId: sessionUserId } = await auth()

-  if (!userId) {
+  if (!sessionUserId) {
     return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
   }
+
+  if (queryUserId && queryUserId !== sessionUserId) {
+    return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
+  }
+
+  const userId = queryUserId ?? sessionUserId
📝 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.

Suggested change
// 1. Identificazione utente
const session = await auth()
const userId = queryUserId || session?.userId
if (!userId) {
const session = await auth()
userId = session.userId
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
// 1. Identificazione utente
const { userId: sessionUserId } = await auth()
if (!sessionUserId) {
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
}
if (queryUserId && queryUserId !== sessionUserId) {
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
}
const userId = queryUserId ?? sessionUserId
🤖 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 10 - 15, The code currently trusts
queryUserId before confirming authentication; call auth() first and if session
is missing immediately return 401, then derive userId only from session.userId
(do not allow queryUserId to override it). Locate the auth() call and the
variables queryUserId, session and userId in route.ts and change the assignment
so userId = session.userId (ignore or validate queryUserId only after verifying
the session and explicit authorization, e.g. admin check in a separate branch).

}
if (!userId) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })

try {
await connectDB()

let teacher = await Teacher.findOne({ clerkId: userId }).lean()

if (!teacher) {
const clerkUser = await currentUser()
const created = await Teacher.create({

if (!clerkUser) {
return NextResponse.json({ error: 'User data not found' }, { status: 404 })
}

const newTeacher = await Teacher.create({
clerkId: userId,
name: clerkUser?.fullName ?? '',
email: clerkUser?.emailAddresses[0]?.emailAddress ?? '',
name: `${clerkUser.firstName ?? ''} ${clerkUser.lastName ?? ''}`.trim() || 'Anonymous',
email: clerkUser.emailAddresses[0]?.emailAddress ?? '',
department: '',
subjects: [],
})
teacher = created.toObject()

teacher = JSON.parse(JSON.stringify(newTeacher))
Comment on lines 21 to +38
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Make first-profile creation atomic.

The findOne → create sequence can race for a new user. With the unique clerkId constraint in models/Teacher.ts, concurrent first GETs can produce a duplicate-key error and return 500.

Prefer an atomic upsert or handle duplicate-key retry:

🛠️ Proposed direction 
-      const newTeacher = await Teacher.create({
-        clerkId: userId,
-        name: `${clerkUser.firstName ?? ''} ${clerkUser.lastName ?? ''}`.trim() || 'Anonymous',
-        email: clerkUser.emailAddresses[0]?.emailAddress ?? '',
-        department: '',
-        subjects: [],
-      })
-      
-      teacher = JSON.parse(JSON.stringify(newTeacher))
+      teacher = await Teacher.findOneAndUpdate(
+        { clerkId: userId },
+        {
+          $setOnInsert: {
+            clerkId: userId,
+            name: `${clerkUser.firstName ?? ''} ${clerkUser.lastName ?? ''}`.trim() || 'Anonymous',
+            email: clerkUser.emailAddresses[0]?.emailAddress ?? '',
+            department: '',
+            subjects: [],
+          },
+        },
+        { new: true, upsert: true, setDefaultsOnInsert: true },
+      ).lean()
📝 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.

Suggested change
let teacher = await Teacher.findOne({ clerkId: userId }).lean()
if (!teacher) {
const clerkUser = await currentUser()
const created = await Teacher.create({
if (!clerkUser) {
return NextResponse.json({ error: 'User data not found' }, { status: 404 })
}
const newTeacher = await Teacher.create({
clerkId: userId,
name: clerkUser?.fullName ?? '',
email: clerkUser?.emailAddresses[0]?.emailAddress ?? '',
name: `${clerkUser.firstName ?? ''} ${clerkUser.lastName ?? ''}`.trim() || 'Anonymous',
email: clerkUser.emailAddresses[0]?.emailAddress ?? '',
department: '',
subjects: [],
})
teacher = created.toObject()
teacher = JSON.parse(JSON.stringify(newTeacher))
let teacher = await Teacher.findOne({ clerkId: userId }).lean()
if (!teacher) {
const clerkUser = await currentUser()
if (!clerkUser) {
return NextResponse.json({ error: 'User data not found' }, { status: 404 })
}
teacher = await Teacher.findOneAndUpdate(
{ clerkId: userId },
{
$setOnInsert: {
clerkId: userId,
name: `${clerkUser.firstName ?? ''} ${clerkUser.lastName ?? ''}`.trim() || 'Anonymous',
email: clerkUser.emailAddresses[0]?.emailAddress ?? '',
department: '',
subjects: [],
},
},
{ new: true, upsert: true, setDefaultsOnInsert: true },
).lean()
🤖 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 21 - 38, The current findOne → create
flow can race and hit the unique clerkId constraint; replace it with an atomic
upsert or retry-on-duplicate pattern: use Teacher.findOneAndUpdate({ clerkId:
userId }, { $setOnInsert: { clerkId: userId, name: ..., email: ..., department:
'', subjects: [] } }, { upsert: true, new: true, setDefaultsOnInsert: true })
(or wrap Teacher.create in a try/catch that retries a findOne after E11000
duplicate-key error) so that the logic around Teacher.findOne, Teacher.create
and assignment to the teacher variable becomes atomic and race-safe while still
using currentUser() to populate defaults.

}

return NextResponse.json(teacher)
} catch (error) {
console.error('GET /api/profile error:', error instanceof Error ? error.message : error)
console.error('GET /api/profile error:', error)
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
}
}
Expand Down
13 changes: 7 additions & 6 deletions app/dashboard/grades/GradesClient.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,14 @@ export function GradesClient() {
() =>
subjects.map((subject) => {
const sg = gradesBySubject[subject];
const avg =
let avg =
sg.length > 0
? Math.round(
sg.reduce((s, g) => s + pct(g.marks, g.maxMarks), 0) /
sg.length,
)
: 0;
?
sg.reduce((s, g) => s + pct(g.marks, g.maxMarks), 0) / sg.length
: 0

avg = Number(avg.toFixed(2));

return {
subject: subject.length > 10 ? subject.slice(0, 10) + "…" : subject,
avg,
Expand Down
Loading