refactor: activities results endpoints & clean code by SonarScan results

[l7wei]

No description provided.

[gemini-code-assist]

Code Review

This pull request refactors several components and API routes to improve type safety, performance, and code organization. Key changes include wrapping component props in Readonly, replacing window with globalThis for better environment compatibility, optimizing React keys, and extracting logic into helper functions or variables. Additionally, a new activityVoterService was introduced to centralize voter count logic. Feedback was provided regarding the description field in the activity update API, which should explicitly support null values to allow clearing the field in the database. There is also a recommendation to use stable unique identifiers for React keys in the OptionFormSection to avoid rendering issues when editable fields change.

[gemini-code-assist]

The description field should allow null to support clearing the value in the database. Without null, the field can only be updated to a new string or left unchanged.

Suggested change

	description?: string;
	description?: string | null;

[gemini-code-assist]

The manual type check for description should include null to allow the field to be cleared. Currently, if rawBody.description is null, it is mapped to undefined, which prevents the update from reaching the database. This is a regression from the previous implementation where the raw body was destructured directly.

Suggested change

	description:
	typeof rawBody.description === "string" ? rawBody.description : undefined,
	description:
	(typeof rawBody.description === "string" || rawBody.description === null) ? rawBody.description : undefined,

[gemini-code-assist]

Using a key derived from editable fields like label and candidate.name is fragile. If two options have the same name and an empty label, the keys will collide, causing React rendering issues. Furthermore, any update to these fields will cause the entire component to unmount and remount because its key changed. It is recommended to use a stable unique identifier for each option in the local state.

[Copilot]

Pull request overview

This PR focuses on refactoring activity/voting-related endpoints and applying a set of SonarScan-driven cleanups across UI and API code (mostly tightening typings, simplifying conditionals, and improving React list key stability in some places).

Changes:

• Introduces a shared getEligibleVotersCount() helper and switches eligible-voter counting to use ActivityVoter documents.
• Refactors several UI components/pages for cleaner JSX, safer globals (globalThis), and Readonly props typing.
• Improves various list renderings (some keys updated) and simplifies conditional logic in API/UI flows.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
lib/statisticsService.ts	Uses shared eligible-voter counting instead of reading from activity.users.
lib/activityVoterService.ts	Adds getEligibleVotersCount() helper around ActivityVoter.countDocuments().
components/ui/loader.tsx	Refactors component signatures and props typing.
components/ui/card.tsx	Tightens CardTitle typing and renders children explicitly.
components/MarkdownRenderer.tsx	Extracts markdown components config and refactors schema spreading.
components/LoginModal.tsx	Uses globalThis.location instead of window.location; Readonly props.
components/Header.tsx	Refactors auth rendering into authContent; uses globalThis.location.
components/ActivityStatusBadge.tsx	Readonly props refactor.
app/vote/certificate/page.tsx	Uses globalThis for print/confirm, improves keys, tweaks global style string.
app/vote/[id]/page.tsx	Simplifies conditionals and updates several list keys and mappings.
app/vote/[id]/completion/page.tsx	Extracts next-step JSX into a reusable variable.
app/verify/page.tsx	Fixes success-branch logic and updates list keys.
app/login/page.tsx	Uses globalThis.location for redirect.
app/layout.tsx	Makes layout props Readonly.
app/api/votes/route.ts	Consolidates validation imports.
app/api/mock/resource/route.ts	Uses optional chaining for auth header parsing.
app/api/mock/authorize/page.tsx	Uses Set for scope checks and switches to globalThis.location.
app/api/activities/[id]/voters/route.ts	Uses shared eligible-voter counting and refactors transaction branching.
app/api/activities/[id]/route.ts	Adds body parsing/coercion helpers and refactors update-data building.
app/admin/settings/page.tsx	Extracts admin list JSX into a variable for readability.
app/admin/activities/_components/useOptionForm.ts	Simplifies add/update option flow.
app/admin/activities/_components/ViceCandidateSection.tsx	Readonly props + changes React list keys for vices.
app/admin/activities/_components/OptionFormSection.tsx	Refactors title logic, keys, and re-exports useOptionForm.
app/admin/activities/_components/CandidateFormFields.tsx	Readonly props refactor.
app/admin/activities/_components/ActivityFormFields.tsx	Readonly props refactor.
app/admin/activities/[id]/verification/page.tsx	Uses globalThis.location and link.remove().
app/admin/activities/[id]/page.tsx	Refactors upload success branching and updates vice list rendering/keys.
Dockerfile	Combines RUN layers for smaller image and fewer layers.

Comments suppressed due to low confidence (2)

app/vote/[id]/page.tsx:266

• Using the experience text in the React key can collide when the same string appears more than once (or when candidate names repeat), which breaks React’s list diffing. Consider including the array index (or a stable id) in the key to guarantee uniqueness.

                  {candidate.personal_experiences.map(
                    (exp: string) => (
                      <li key={`${candidate.name}-exp-${exp}`} className="text-sm">
                        <MarkdownRenderer
                          content={exp}
                          className="break-words"
                        />
                      </li>
                    ),

app/vote/[id]/page.tsx:292

• The opinion text is used as the React key, which can collide if the same opinion string appears multiple times. Use a key that’s guaranteed unique within the list (e.g., include the array index or a stable id).

                  {candidate.political_opinions.map(
                    (opinion: string) => (
                      <li
                        key={`${candidate.name}-opinion-${opinion}`}
                        className="text-sm"
                      >
                        <MarkdownRenderer
                          content={opinion}
                          className="break-words"
                        />
                      </li>
                    ),
                  )}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

selection is used as the React list key, which can collide if the same selection string appears more than once. Use a key that is guaranteed unique (e.g., include the index or a stable id).

Suggested change

	{result.selections.map((selection) => (
	<li key={selection}>{selection}</li>
	{result.selections.map((selection, index) => (
	<li key={`${selection}-${index}`}>{selection}</li>

[Copilot]

The vice list item key is derived from vice fields that can be missing or duplicated. Duplicate keys can cause incorrect rendering/state reuse. Consider including viceIndex (or another stable unique identifier) in the key.

Suggested change

	key={`${option._id}-${vice.name}-${vice.department}-${vice.college}`}
	key={`${option._id}-${viceIndex}-${vice.name}-${vice.department}-${vice.college}`}

[Copilot]

This route now contains non-trivial request-body parsing/coercion and date/rule validation logic. The repo has Jest coverage for other API routes, but this handler doesn’t appear to be tested; adding unit tests for PUT (valid/invalid rule, invalid dates, partial updates) would help prevent regressions.

[Copilot]

The key is derived from editable vice fields (name/department/college). When multiple vices are newly added (often empty strings/undefined), this can produce duplicate keys and cause React to reuse DOM/state between items. Use a stable unique key per vice entry (e.g., include index, or generate an id when adding a vice).

Suggested change

	key={`${vice.name}-${vice.department}-${vice.college}`}
	key={`vice-${index}-${vice.name ?? ""}-${vice.department ?? ""}-${vice.college ?? ""}`}

[Copilot]

This list item key is built from label (optional) and candidate/vice names. It can easily collide (e.g., undefined label, duplicate names, empty vice names), leading to React key duplication and rendering/state issues. Prefer a stable unique key (e.g., include index or assign a per-option id when creating options).

Suggested change

	key={`${option.label}-${option.candidate.name}-${option.vice
	.map((v) => v.name)
	.join("-")}`}
	key={`option-${index}`}

[Copilot]

The vice-candidate wrapper div key is derived from vice fields that may be undefined or duplicated across entries. This can produce duplicate keys and unstable UI updates. Consider including the map index or a generated id to ensure uniqueness.

Suggested change

	{option.vice?.map((vice) => (
	<div key={`${option._id}-${vice.name}-${vice.department}-${vice.college}`}>
	{renderCandidate(vice)}
	</div>
	))}
	{option.vice?.map((vice, viceIndex) => (
	<div
	key={`${option._id}-${vice.name}-${vice.department}-${vice.college}-${viceIndex}`}
	>
	{renderCandidate(vice)}
	</div>
	))}

[l7wei]

@copilot apply changes based on the comments in this thread and this thread

[Copilot]

@copilot apply changes based on the comments in this thread and [this thread](https://github.com/NTHU-SA/Voting-System/...

已依兩個 review thread 完成修正，包含 key 穩定性調整與 /api/activities/[id] PUT 測試補齊（valid partial update / invalid rule / invalid date range）。對應提交：0805d6a。UI 變更截圖：https://github.com/user-attachments/assets/bea04269-f827-4cbc-8279-9108d9e35b2a

[sonarqubecloud]

Quality Gate failed

Failed conditions
6.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud