feat: update candidate schema html support / add 驗票 - #47
Conversation
Agent-Logs-Url: https://github.com/NTHU-SA/Voting-System/sessions/d7f1c360-ef7c-43b5-a6ab-18bcc0cd81d6 Co-authored-by: l7wei <87221670+l7wei@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NTHU-SA/Voting-System/sessions/d7f1c360-ef7c-43b5-a6ab-18bcc0cd81d6 Co-authored-by: l7wei <87221670+l7wei@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates voter and administrator management from static CSV files to a MongoDB-backed system, introducing a ROOT_ADMIN role and activity-specific voter rosters. Key additions include a public vote verification page, candidate descriptions with Markdown support, and administrative APIs for managing users and voter lists. Feedback identifies a scalability concern in the vote verification API regarding the lack of pagination and provides a suggestion to fix indentation in the admin activity dashboard.
| const votes = await Vote.find({ activity_id: id }) | ||
| .select("token created_at") | ||
| .select("token created_at rule choose_one choose_all") | ||
| .sort({ created_at: -1 }) | ||
| .lean(); |
There was a problem hiding this comment.
Fetching all votes for an activity without any limit or pagination can lead to significant performance issues and memory exhaustion if an activity has a large number of votes (e.g., thousands). Consider implementing pagination or at least a reasonable upper limit for this admin verification route.
| candidate: { | ||
| name: option.candidate?.name || "", | ||
| department: option.candidate?.department || "", | ||
| college: option.candidate?.college || "", | ||
| avatar_url: option.candidate?.avatar_url || "", | ||
| description: option.candidate?.description || "", | ||
| experiences: option.candidate?.personal_experiences?.join("\n") || "", | ||
| opinions: option.candidate?.political_opinions?.join("\n") || "", | ||
| }, | ||
| vice: (option.vice || []).map((v) => ({ | ||
| name: v.name || "", | ||
| department: v.department || "", | ||
| college: v.college || "", | ||
| avatar_url: v.avatar_url || "", | ||
| description: v.description || "", | ||
| experiences: v.personal_experiences?.join("\n") || "", | ||
| opinions: v.political_opinions?.join("\n") || "", | ||
| })), | ||
| }); |
There was a problem hiding this comment.
The indentation for the candidate and vice properties is inconsistent with the rest of the object structure. It should be aligned with the label property for better readability.
| candidate: { | |
| name: option.candidate?.name || "", | |
| department: option.candidate?.department || "", | |
| college: option.candidate?.college || "", | |
| avatar_url: option.candidate?.avatar_url || "", | |
| description: option.candidate?.description || "", | |
| experiences: option.candidate?.personal_experiences?.join("\n") || "", | |
| opinions: option.candidate?.political_opinions?.join("\n") || "", | |
| }, | |
| vice: (option.vice || []).map((v) => ({ | |
| name: v.name || "", | |
| department: v.department || "", | |
| college: v.college || "", | |
| avatar_url: v.avatar_url || "", | |
| description: v.description || "", | |
| experiences: v.personal_experiences?.join("\n") || "", | |
| opinions: v.political_opinions?.join("\n") || "", | |
| })), | |
| }); | |
| candidate: { | |
| name: option.candidate?.name || "", | |
| department: option.candidate?.department || "", | |
| college: option.candidate?.college || "", | |
| avatar_url: option.candidate?.avatar_url || "", | |
| description: option.candidate?.description || "", | |
| experiences: option.candidate?.personal_experiences?.join("\n") || "", | |
| opinions: option.candidate?.political_opinions?.join("\n") || "", | |
| }, | |
| vice: (option.vice || []).map((v) => ({ | |
| name: v.name || "", | |
| department: v.department || "", | |
| college: v.college || "", | |
| avatar_url: v.avatar_url || "", | |
| description: v.description || "", | |
| experiences: v.personal_experiences?.join("\n") || "", | |
| opinions: v.political_opinions?.join("\n") || "", | |
| })), |
There was a problem hiding this comment.
Pull request overview
This PR expands the voting system to support richer candidate content (Markdown/HTML), introduces a public UUID-based vote verification flow (“公開驗票”), and migrates admin/eligible-voter management from CSV files to MongoDB with a ROOT_ADMIN concept.
Changes:
- Add Markdown/HTML rendering for candidate description/experiences/opinions and loosen CSP to allow external images.
- Add public verification UI + API (
/verify,/api/verify/:uuid) and enrich admin verification/vote listing with human-readable selections. - Move admin list and per-activity voter rosters into MongoDB (new
admins+activity_votersmodels/APIs, new admin settings page).
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| types/index.ts | Adds new fields/types for root admin + admin/voter collections + candidate description. |
| proxy.ts | Marks verify routes as public and broadens CSP img-src for external images. |
| package.json | Adds react-markdown + rehype plugins for Markdown/HTML rendering. |
| package-lock.json | Locks new Markdown-related dependencies. |
| lib/voterList.ts | Removes CSV-based voter list helper. |
| lib/models/Option.ts | Extends candidate schema with description. |
| lib/models/Admin.ts | New MongoDB model for administrators. |
| lib/models/ActivityVoter.ts | New MongoDB model for per-activity eligible voters (with indexes). |
| lib/models/Activity.ts | Adds eligible voter count field to activity schema. |
| lib/auth.ts | Reworks admin checks to use ROOT_ADMIN + admins collection. |
| hooks/useUser.ts | Surfaces isRootAdmin on the client user model. |
| components/MarkdownRenderer.tsx | New client component for Markdown + sanitized raw HTML rendering. |
| components/Header.tsx | Adds “公開驗票” nav item + root-admin-only settings link. |
| app/vote/[id]/page.tsx | Renders candidate content via MarkdownRenderer (incl. new description section). |
| app/verify/page.tsx | New public UI to verify a vote by UUID. |
| app/api/votes/route.ts | Eligibility now checks ActivityVoter; GET now enriches votes with readable selections. |
| app/api/verify/[token]/route.ts | New public verification endpoint that returns selections for a UUID token. |
| app/api/auth/check/route.ts | Includes isRootAdmin in auth check response. |
| app/api/admins/route.ts | New root-admin-only CRUD API for admins. |
| app/api/activities/route.ts | Initializes eligibleVotersCount on activity creation. |
| app/api/activities/[id]/voters/route.ts | New admin endpoint to upload/replace per-activity eligible voter roster via CSV. |
| app/api/activities/[id]/verification/route.ts | Enriches verification output with selections + activity name. |
| app/admin/settings/page.tsx | New root-admin-only UI to manage admins. |
| app/admin/page.tsx | Adds root-admin shortcut to settings. |
| app/admin/activities/_components/utils.ts | Sends candidate description in payload. |
| app/admin/activities/_components/types.ts | Adds description to candidate form type. |
| app/admin/activities/_components/formHelpers.ts | Initializes empty candidate description. |
| app/admin/activities/_components/CandidateFormFields.tsx | Adds description textarea + updates placeholders for Markdown/HTML support. |
| app/admin/activities/[id]/verification/page.tsx | Displays + exports vote selections in verification UI/CSV. |
| app/admin/activities/[id]/page.tsx | Adds per-activity voter roster upload UI + shows eligible voter count. |
| tests/auth.integration.test.ts | Updates mocks to include isRootAdmin. |
| tests/api.votes.test.ts | Updates vote route tests for DB-based eligibility + enriched GET response. |
| README.md | Updates env/config docs for ROOT_ADMIN and MongoDB-based rosters/admins; documents public verify API. |
| .env.example | Adds ROOT_ADMIN and clarifies environment values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| eligibleVotersCount: { | ||
| type: Number, | ||
| required: true, | ||
| default: 0, | ||
| min: 0, | ||
| }, |
There was a problem hiding this comment.
eligibleVotersCount introduces a camelCase persisted field in Activity while the rest of the schema uses snake_case (open_from, created_at, etc.). This inconsistency makes querying/serialization harder and increases the chance of subtle bugs. Consider renaming the stored field to eligible_voters_count (and updating types/routes/UI accordingly), or adding a schema alias/virtual so both names work consistently.
| export interface IActivity { | ||
| _id: Types.ObjectId | string; | ||
| name: string; | ||
| type: string; // Kept for backward compatibility | ||
| subtitle?: string; | ||
| description?: string; // New field for activity description | ||
| eligibleVotersCount: number; | ||
| rule: "choose_all" | "choose_one"; |
There was a problem hiding this comment.
IActivity now requires eligibleVotersCount, but persisted activity fields elsewhere are snake_case (e.g. open_from, created_at). Aligning the property name with the DB field naming convention will reduce confusion across API payloads and Mongoose queries (see comment in lib/models/Activity.ts).
| return createSuccessResponse({ | ||
| activity_id: id, | ||
| activity_name: "", | ||
| total_votes: 0, | ||
| voted_tokens: [], | ||
| }); |
There was a problem hiding this comment.
When the activity does not exist, this endpoint currently returns a success: true response with empty data. That makes it hard for admin clients to distinguish "no votes yet" vs "invalid activity id" and is inconsistent with other activity endpoints. Consider returning a 404 error (e.g. ACTIVITY_NOT_FOUND) instead.
| return createSuccessResponse({ | |
| activity_id: id, | |
| activity_name: "", | |
| total_votes: 0, | |
| voted_tokens: [], | |
| }); | |
| return NextResponse.json( | |
| { | |
| success: false, | |
| error: "ACTIVITY_NOT_FOUND", | |
| message: "Activity not found", | |
| }, | |
| { status: 404 }, | |
| ); |
|
|
||
| const { token } = await params; | ||
| if (!token?.trim()) { | ||
| return createErrorResponse("UUID is required", 400); |
There was a problem hiding this comment.
The 400 error message is English ("UUID is required") while other responses in this handler/UI are Chinese. For a consistent UX, consider using the same locale for all errors returned by this endpoint.
| return createErrorResponse("UUID is required", 400); | |
| return createErrorResponse("UUID 為必填欄位", 400); |
| <ul className="space-y-1"> | ||
| {candidate.personal_experiences.map( | ||
| (exp: string, idx: number) => ( | ||
| <li key={idx} className="flex items-start text-sm"> | ||
| <span className="mr-2 text-primary">•</span> | ||
| <span>{exp}</span> | ||
| <li key={idx} className="text-sm"> | ||
| <MarkdownRenderer | ||
| content={exp} | ||
| className="break-words" | ||
| /> | ||
| </li> |
There was a problem hiding this comment.
This change removes the explicit bullet marker for experiences/opinions, so items will render without bullets unless the content itself contains Markdown list syntax. If the intent is to keep a list UI, consider restoring bullets via Tailwind list styles (e.g. list-disc list-inside) or reintroducing the marker.
| // GET /api/verify/[token] - Public UUID verification | ||
| export async function GET( | ||
| _request: NextRequest, | ||
| { params }: { params: Promise<{ token: string }> }, | ||
| ) { | ||
| try { | ||
| await connectDB(); | ||
|
|
||
| const { token } = await params; | ||
| if (!token?.trim()) { | ||
| return createErrorResponse("UUID is required", 400); | ||
| } | ||
|
|
||
| const vote = await Vote.findOne({ token: token.trim() }).lean(); | ||
| if (!vote) { | ||
| return createErrorResponse("找不到此 UUID 的投票記錄", 404); | ||
| } | ||
|
|
||
| const activity = await Activity.findById(vote.activity_id) | ||
| .select("name") | ||
| .lean(); | ||
| const options = await Option.find({ activity_id: vote.activity_id }) | ||
| .select("_id label candidate.name") | ||
| .lean(); | ||
|
|
||
| const optionMap = new Map<string, string>(); | ||
| options.forEach((option) => { | ||
| optionMap.set( | ||
| option._id.toString(), | ||
| option.label || option.candidate?.name || option._id.toString(), | ||
| ); | ||
| }); | ||
|
|
||
| const selections = | ||
| vote.rule === "choose_one" | ||
| ? [ | ||
| optionMap.get(vote.choose_one?.toString() || "") || | ||
| vote.choose_one?.toString() || | ||
| "", | ||
| ].filter(Boolean) | ||
| : (vote.choose_all || []).map((choice) => { | ||
| const optionName = | ||
| optionMap.get(choice.option_id.toString()) || | ||
| choice.option_id.toString(); | ||
| return `${optionName}（${choice.remark}）`; | ||
| }); | ||
|
|
||
| return createSuccessResponse({ | ||
| uuid: vote.token, | ||
| activity_id: vote.activity_id, | ||
| activity_name: activity?.name || "未知活動", | ||
| voted_at: vote.created_at, | ||
| rule: vote.rule, | ||
| selections, | ||
| }); |
There was a problem hiding this comment.
This new public verification endpoint has several branching behaviors (400 for missing UUID, 404 for unknown UUID, and success mapping of selections). The repo already has Jest coverage for other API routes (e.g. /api/votes), so adding route tests here would help prevent regressions and ensure the anonymized selection mapping stays correct.
| async function requireRootAdmin(request: NextRequest) { | ||
| const authResult = await requireAuth(request); | ||
| if (authResult instanceof NextResponse) return authResult; | ||
| if (!isRootAdmin(authResult.student_id)) { | ||
| return createErrorResponse(API_CONSTANTS.ERRORS.ADMIN_REQUIRED, 403); | ||
| } | ||
| return authResult; | ||
| } | ||
|
|
||
| // GET /api/admins - list admins (ROOT_ADMIN only) | ||
| export async function GET(request: NextRequest) { | ||
| try { | ||
| const authResult = await requireRootAdmin(request); | ||
| if (authResult instanceof NextResponse) { | ||
| return authResult; | ||
| } | ||
|
|
||
| await connectDB(); | ||
| const rootAdmin = getRootAdminStudentId(); | ||
| const admins = await Admin.find().sort({ created_at: -1 }).lean(); | ||
|
|
||
| return createSuccessResponse({ | ||
| root_admin: rootAdmin, | ||
| admins, | ||
| }); | ||
| } catch (error: unknown) { | ||
| return createInternalErrorResponse( | ||
| error, | ||
| "Failed to get admins", | ||
| "Get admins error", | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // POST /api/admins - create admin (ROOT_ADMIN only) | ||
| export async function POST(request: NextRequest) { | ||
| try { | ||
| const authResult = await requireRootAdmin(request); | ||
| if (authResult instanceof NextResponse) { | ||
| return authResult; | ||
| } | ||
|
|
||
| await connectDB(); | ||
| const body = await request.json(); | ||
| const studentId = body.student_id?.trim(); | ||
| const name = body.name?.trim() || undefined; | ||
|
|
||
| if (!studentId) { | ||
| return createErrorResponse( | ||
| `${API_CONSTANTS.ERRORS.MISSING_FIELD}: student_id`, | ||
| ); | ||
| } | ||
|
|
||
| if (isRootAdmin(studentId)) { | ||
| return createErrorResponse("ROOT_ADMIN is managed by environment variable"); | ||
| } | ||
|
|
||
| const admin = await Admin.findOneAndUpdate( | ||
| { student_id: studentId }, | ||
| { | ||
| $set: { | ||
| student_id: studentId, | ||
| name, | ||
| updated_at: new Date(), | ||
| }, | ||
| $setOnInsert: { | ||
| created_at: new Date(), | ||
| }, | ||
| }, | ||
| { | ||
| upsert: true, | ||
| new: true, | ||
| runValidators: true, | ||
| }, | ||
| ); | ||
|
|
||
| return createSuccessResponse(admin, 201); | ||
| } catch (error: unknown) { | ||
| return createInternalErrorResponse( | ||
| error, | ||
| "Failed to create admin", | ||
| "Create admin error", | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // DELETE /api/admins?student_id=xxx - delete admin (ROOT_ADMIN only) | ||
| export async function DELETE(request: NextRequest) { | ||
| try { | ||
| const authResult = await requireRootAdmin(request); | ||
| if (authResult instanceof NextResponse) { | ||
| return authResult; | ||
| } | ||
|
|
||
| await connectDB(); | ||
| const studentId = request.nextUrl.searchParams.get("student_id")?.trim(); | ||
| if (!studentId) { | ||
| return createErrorResponse( | ||
| `${API_CONSTANTS.ERRORS.MISSING_FIELD}: student_id`, | ||
| ); | ||
| } | ||
|
|
||
| if (isRootAdmin(studentId)) { | ||
| return createErrorResponse("Cannot delete ROOT_ADMIN", 400); | ||
| } | ||
|
|
||
| const deleted = await Admin.findOneAndDelete({ student_id: studentId }); | ||
| if (!deleted) { | ||
| return createErrorResponse("Admin not found", 404); | ||
| } | ||
|
|
||
| return createSuccessResponse({ message: "Admin deleted successfully" }); | ||
| } catch (error: unknown) { | ||
| return createInternalErrorResponse( | ||
| error, | ||
| "Failed to delete admin", | ||
| "Delete admin error", | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
This new root-admin management API (GET/POST/DELETE) introduces permission checks and upsert/delete behavior. Given the repo’s existing Jest coverage for auth/API routes, it would be good to add tests covering: root-only access enforcement, upsert behavior, and 404 on delete of missing admin.
| a: (props) => { | ||
| const target = props.target || "_blank"; | ||
| const rel = target === "_blank" ? "noopener noreferrer" : props.rel; | ||
| return <a {...props} target={target} rel={rel} />; |
There was a problem hiding this comment.
In the custom a renderer, props from react-markdown includes non-DOM fields (e.g. node). Spreading them onto <a> can produce React "unknown prop" warnings and leak unintended attributes into the DOM. Destructure out non-DOM props (at least node) before spreading, and only pass valid anchor props through.
| a: (props) => { | |
| const target = props.target || "_blank"; | |
| const rel = target === "_blank" ? "noopener noreferrer" : props.rel; | |
| return <a {...props} target={target} rel={rel} />; | |
| a: ({ node, target, rel, ...anchorProps }) => { | |
| const resolvedTarget = target || "_blank"; | |
| const resolvedRel = | |
| resolvedTarget === "_blank" ? "noopener noreferrer" : rel; | |
| return <a {...anchorProps} target={resolvedTarget} rel={resolvedRel} />; |
Agent-Logs-Url: https://github.com/NTHU-SA/Voting-System/sessions/e5545cfe-2b10-4ca0-b757-6e211bda05ac Co-authored-by: l7wei <87221670+l7wei@users.noreply.github.com>
|
No description provided.