-
Notifications
You must be signed in to change notification settings - Fork 49
feat: Report tag #419
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.
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
base: staging
Are you sure you want to change the base?
feat: Report tag #419
Conversation
added a complete tag-reporting system->a backend API with validation + rate limiting, a new DB model (tagreports) to store reports, and a frontend modal. Removed page up and down buttons , replaced by report Flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a comprehensive tag reporting system that allows users to flag incorrect metadata (subject, exam, slot, year, course code) for papers directly from the PDF viewer. The implementation includes a backend API with validation and rate limiting, a MongoDB model for storing reports, and a user-friendly modal interface.
Key Changes
- Created a complete reporting workflow with
/api/report-tagendpoint featuring field validation, IP-based rate limiting, and per-paper report limits - Built a
ReportTagModalcomponent with multi-select incorrect fields, smart value pre-population, and client-side validation - Streamlined PDF viewer UI by removing page navigation buttons and adding a prominent red report flag button
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
src/db/tagReport.ts |
New MongoDB schema for storing tag reports with fields for paper ID, reported fields, comments, and resolution status |
src/app/api/report-tag/route.ts |
Backend API endpoint with validation logic, rate limiting (3 reports/IP/hour, 2 reports/paper max), and exam/year format validation |
src/components/ReportTagModal.tsx |
Modal component allowing users to select incorrect fields, provide corrections, and submit reports with validation |
src/components/ReportButton.tsx |
Simple button component that triggers the report modal with red flag styling |
src/components/pdfViewer.tsx |
Updated to remove page navigation buttons, disable text/annotation layers, and integrate report button in mobile and desktop toolbars |
src/app/paper/[id]/page.tsx |
Passes paper metadata (paperId, subject, exam, slot, year) to PdfViewer for reporting functionality |
Comments suppressed due to low confidence (2)
src/components/pdfViewer.tsx:26
- Unused variable reportOpen.
const [reportOpen, setReportOpen] = useState(false);
src/components/pdfViewer.tsx:26
- Unused variable setReportOpen.
const [reportOpen, setReportOpen] = useState(false);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/app/api/report-tag/route.ts
Outdated
|
|
||
| const ipCounters = new Map<string, { count: number; resetAt: number }>(); |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ipCounters Map will grow indefinitely and never clear expired entries, causing a memory leak over time. Implement a cleanup mechanism to periodically remove expired entries or use a time-based cache with automatic expiration (e.g., node-cache or lru-cache).
| const ipCounters = new Map<string, { count: number; resetAt: number }>(); | |
| import NodeCache from "node-cache"; | |
| const ipCounters = new NodeCache({ stdTTL: IP_WINDOW_MS / 1000, checkperiod: 60 }); |
src/components/ReportTagModal.tsx
Outdated
| </div> | ||
| ); | ||
| } | ||
| else if(c=="year"){ |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use strict equality operator (===) instead of loose equality (==) for consistency and to avoid type coercion issues.
| else if(c=="year"){ | |
| else if(c === "year"){ |
src/app/api/report-tag/route.ts
Outdated
| function getClientIp(req: Request) { | ||
| const forwarded = req.headers.get("x-forwarded-for"); | ||
| if (forwarded) { | ||
| const first = forwarded.split(",")[0] ?? forwarded; | ||
| return first.trim(); | ||
| } | ||
| const real = req.headers.get("x-real-ip"); | ||
| if (real) return real; | ||
| try { | ||
| const url = new URL(req.url); | ||
| return url.hostname || "127.0.0.1"; | ||
| } catch { | ||
| return "127.0.0.1"; | ||
| } |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getClientIp function can be easily spoofed since it relies on x-forwarded-for and x-real-ip headers which can be manipulated by clients. This undermines the rate limiting mechanism. Consider implementing additional validation or using a more robust IP detection method, especially if not behind a trusted proxy.
| function getClientIp(req: Request) { | |
| const forwarded = req.headers.get("x-forwarded-for"); | |
| if (forwarded) { | |
| const first = forwarded.split(",")[0] ?? forwarded; | |
| return first.trim(); | |
| } | |
| const real = req.headers.get("x-real-ip"); | |
| if (real) return real; | |
| try { | |
| const url = new URL(req.url); | |
| return url.hostname || "127.0.0.1"; | |
| } catch { | |
| return "127.0.0.1"; | |
| } | |
| // WARNING: Cannot reliably determine client IP in this environment. | |
| // The use of x-forwarded-for and x-real-ip headers is insecure unless behind a trusted proxy. | |
| // For secure rate limiting, ensure your deployment is behind a trusted proxy that sets these headers, | |
| // and use a server adapter that exposes the remote address (e.g., req.socket.remoteAddress). | |
| // Here, we return a static value to avoid spoofing. | |
| function getClientIp(req: Request) { | |
| return "unknown"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of using "x-forwarded-for" and "x-real-ip"(which can be spoofed) --> req.ip is being used to get the actual ip
Additionally
added a per-paper report limit, so even if someone tries to bypass the IP limit, each paper can only receive a small fixed number of reports.
src/app/api/report-tag/route.ts
Outdated
| import { connectToDatabase } from "@/lib/database/mongoose"; | ||
| import TagReport from "@/db/tagReport"; | ||
|
|
||
| const ipCounters = new Map<string, { count: number; resetAt: number }>(); |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The in-memory rate limiting using Map will be reset when the server restarts or in serverless environments where each request might run on a different instance. This makes the rate limiting unreliable. Consider using a persistent storage solution like Redis or a database-backed rate limiting approach for production reliability.
src/components/ReportTagModal.tsx
Outdated
| /> | ||
| </div> | ||
| ); | ||
| } else if (c == "exam") { |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use strict equality operator (===) instead of loose equality (==) for consistency and to avoid type coercion issues.
| } else if (c == "exam") { | |
| } else if (c === "exam") { |
src/components/ReportTagModal.tsx
Outdated
| } catch (err: any) { | ||
| console.error(err); | ||
| toast.error(err?.response?.data?.error || "Failed to submit report."); |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handling doesn't properly display API error messages. When the promise is rejected, the error message from err?.response?.data?.error is logged but the toast.promise already shows "Failed to submit report." Consider restructuring to pass the API error message to the toast, or handle the error outside of toast.promise to show the specific error message.
src/app/api/report-tag/route.ts
Outdated
| ); | ||
| } | ||
| continue; | ||
| } |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The year validation logic is incomplete. After checking the year range format, the code continues to the next iteration but doesn't validate single year formats (e.g., "2024"). A single year should also be validated to ensure it falls within the 1900-2100 range. Add validation for single year format after the range check.
| } | |
| } | |
| // Validate single year format | |
| const singleYearMatch = val.match(/^(\d{4})$/); | |
| if (singleYearMatch) { | |
| const year = Number(singleYearMatch[1]); | |
| if (year < 1900 || year > 2100) { | |
| return NextResponse.json( | |
| { error: `Invalid year: ${rf.value}` }, | |
| { status: 400 } | |
| ); | |
| } | |
| continue; | |
| } | |
| // If not a valid range or single year, reject | |
| return NextResponse.json( | |
| { error: `Invalid year format: ${rf.value}` }, | |
| { status: 400 } | |
| ); |
src/components/ReportTagModal.tsx
Outdated
| else if (c === "year" && year) | ||
| setCategoryValues((s) => ({ ...s, [c]: year })); | ||
| } | ||
| }, [selectedCategories]); |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing dependency in the useEffect dependency array. The effect depends on subject, exam, slot, and year but they're not included in the dependency array. This could lead to stale values being used when these props change. Add these props to the dependency array: [selectedCategories, subject, exam, slot, year].
| }, [selectedCategories]); | |
| }, [selectedCategories, subject, exam, slot, year]); |
src/components/ReportTagModal.tsx
Outdated
| const [originalCategoryValues, setOriginalCategoryValues] = useState< | ||
| Record<string, string> | ||
| >({}); | ||
| const [originalComment, setOriginalComment] = useState(""); |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable originalComment.
| const [originalComment, setOriginalComment] = useState(""); |
| Record<string, string> | ||
| >({}); | ||
| const [originalComment, setOriginalComment] = useState(""); | ||
| const [originalEmail, setOriginalEmail] = useState(""); |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable originalEmail.
| const [originalEmail, setOriginalEmail] = useState(""); |
| <button className="rounded-full border border-red-300 p-2 text-red-500 transition hover:border-red-500 hover:text-red-600 hover:shadow-[0_0_8px_rgba(255,0,0,0.4)]"> | ||
| <FaFlag className="text-sm" /> | ||
| </button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use shadCN components for consistency. EG here use Button instead of button
src/components/ReportTagModal.tsx
Outdated
| if (reportedFields.length === 0) { | ||
| toast.error("You haven’t changed anything to report."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just disable the button if user haven't changed anything right?
| <div key={c} className="w-full"> | ||
| <label className="mb-1 block text-sm"> | ||
| Subject name | ||
| </label> | ||
| <Input | ||
| value={categoryValues["subject"] ?? ""} | ||
| onChange={(e) => | ||
| setCategoryValues((s) => ({ | ||
| ...s, | ||
| subject: e.target.value, | ||
| })) | ||
| } | ||
| placeholder="Subject name" | ||
| className="mb-2 w-full" | ||
| /> | ||
| <label className="mb-1 block text-sm"> | ||
| Course code (optional) | ||
| </label> | ||
| <Input | ||
| value={categoryValues["courseCode"] ?? ""} | ||
| onChange={(e) => | ||
| setCategoryValues((s) => ({ | ||
| ...s, | ||
| courseCode: e.target.value, | ||
| })) | ||
| } | ||
| placeholder="e.g. BMAT205L" | ||
| className="w-full" | ||
| /> | ||
| </div> | ||
| ); | ||
| } else if (c == "exam") { | ||
| return ( | ||
| <div key={c} className="w-full"> | ||
| <label className="mb-1 block text-sm capitalize"> | ||
| Exam | ||
| </label> | ||
|
|
||
| <select | ||
| value={categoryValues["exam"] ?? ""} | ||
| onChange={(e) => | ||
| setCategoryValues((s) => ({ | ||
| ...s, | ||
| exam: e.target.value, | ||
| })) | ||
| } | ||
| className="w-full rounded border bg-white p-2 dark:bg-[#1f1f2a]" | ||
| > | ||
| <option value="CAT-1">CAT-1</option> | ||
| <option value="CAT-2">CAT-2</option> | ||
| <option value="FAT">FAT</option> | ||
| </select> | ||
| </div> | ||
| ); | ||
| } else if (c == "slot") { | ||
| return ( | ||
| <div key={c} className="w-full"> | ||
| <label className="mb-1 block text-sm capitalize"> | ||
| Slot | ||
| </label> | ||
| <Input | ||
| value={categoryValues[c] ?? ""} | ||
| maxLength={2} | ||
| onChange={(e) => { | ||
| let v = e.target.value.toUpperCase(); | ||
| v = v.replace(/[^A-Z0-9]/g, ""); | ||
|
|
||
| setCategoryValues((s) => ({ ...s, slot: v })); | ||
| }} | ||
| placeholder="e.g. D1" | ||
| className="w-full" | ||
| /> | ||
| </div> | ||
| ); | ||
| } | ||
| else if(c=="year"){ | ||
| return ( | ||
| <div key={c} className="w-full"> | ||
| <label className="mb-1 block text-sm capitalize">Year</label> | ||
| <Input | ||
| value={categoryValues[c] ?? ""} | ||
| onChange={(e) => | ||
| setCategoryValues((s) => ({ ...s, year: e.target.value })) | ||
| } | ||
| placeholder="e.g. 2024-2025" | ||
| className="w-full" | ||
| /> | ||
| </div> | ||
| ); | ||
| } | ||
| return ( | ||
| <div key={c} className="w-full"> | ||
| <label className="mb-1 block text-sm capitalize"> | ||
| {c} | ||
| </label> | ||
| <Input | ||
| value={categoryValues[c] ?? ""} | ||
| onChange={(e) => | ||
| setCategoryValues((s) => ({ | ||
| ...s, | ||
| [c]: e.target.value, | ||
| })) | ||
| } | ||
| className="w-full" | ||
| /> | ||
| </div> | ||
| ); | ||
| })} | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| <div> | ||
| <label className="mb-2 block text-sm font-medium"> | ||
| Comment (optional) | ||
| </label> | ||
| <Input | ||
| value={comment} | ||
| onChange={(e) => setComment(e.target.value)} | ||
| placeholder="Short note" | ||
| /> | ||
| </div> | ||
|
|
||
| <div> | ||
| <label className="mb-2 block text-sm font-medium"> | ||
| Your email (optional) | ||
| </label> | ||
| <Input | ||
| value={email} | ||
| onChange={(e) => setEmail(e.target.value)} | ||
| placeholder="you@example.com" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this pattern of taking in Input for and label is being repeated a lot right? Could be that you separate this into reusable function or component?
| renderAnnotationLayer | ||
| renderTextLayer | ||
| renderAnnotationLayer={false} | ||
| renderTextLayer={false} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do this 👀
1) Fixed a few minor issues. 2) Used existing Shadcn components to maintain UI consistency. 3) Implemented Upstash Rate Limiting and set up a Redis database to prevent spoofing(IP) and report spamming.
Added a complete tag-reporting system
Updated PDF Viewer UI
📌 Purpose
It improves data accuracy and provides a structured way to collect user feedback on incorrect tags.
Corresponding issue: closes #284
🖼️ Showcase
🔧 Changes
Added /api/report-tag endpoint with:
Added new Mongoose model TagReport.
Added ReportButton component to trigger the modal.
Added ReportTagModal with:
Cleaned up PDF viewer UI: removed page navigation arrows, added report flag.
No. of reports from an IP, in an hour is set to three and Maximum reports allowed for a paperID in DB is set to two.( Only For Testing).
➕ Additional Notes