fix: evoting reported issues #462
Conversation
WalkthroughThis PR extends eReputation voting by adding pointsVoted and totalEligiblePoints fields to poll results, updates result rendering to conditionally display "Points" instead of "votes" in eReputation mode, implements localStorage-based sort preference persistence, and removes active poll pre-sorting bias from the API layer. Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend as Frontend (page.tsx)
participant API as Backend (VoteService)
participant Storage as Browser Storage
Frontend->>Storage: Read sortField & sortDirection on mount
Storage-->>Frontend: Return preferences (or defaults)
Frontend->>Frontend: Initialize sort state
User->>Frontend: Toggle sort direction
Frontend->>Storage: Persist sortDirection
User->>Frontend: Select new sort field
Frontend->>Storage: Persist sortField & reset direction
User->>Frontend: Request poll results
Frontend->>API: GET poll results with sort params
API->>API: Compute totalEligiblePoints & pointsVoted
API-->>Frontend: Return PollResults {<br/> ...existing fields,<br/> pointsVoted,<br/> totalEligiblePoints<br/>}
Frontend->>Frontend: Render results<br/>(show "Points" if eReputation mode)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platforms/evoting-api/src/services/VoteService.ts (1)
274-297: Guard against missingreputationResults.resultswhen computing totalsThese new aggregates:
if (isWeighted && reputationResults) { totalEligiblePoints = reputationResults.results.reduce((sum, r) => sum + r.score, 0); pointsVoted = totalWeightedVotes; }and
if (reputationResults) { totalEligiblePoints = reputationResults.results.reduce((sum, r) => sum + r.score, 0) * 100; pointsVoted = totalWeightedPoints; }assume
reputationResults.resultsis always a defined array. Elsewhere (getReputationScore) you explicitly guard against!reputationResults.results, returning a default weight instead, which suggests this field can legitimately be absent.To keep
getPollResultsfrom throwing in that case, consider mirroring that guard:if (isWeighted && reputationResults && Array.isArray(reputationResults.results)) { totalEligiblePoints = reputationResults.results.reduce((sum, r) => sum + r.score, 0); pointsVoted = totalWeightedVotes; }and similarly for the points‑mode branch. If
resultsis missing, you can simply omit these optional fields (leave themundefined) and the frontend will behave as before.Also applies to: 355-363
🧹 Nitpick comments (5)
platforms/evoting-api/src/services/PollService.ts (1)
86-127: Hoistnowout of the sort comparator for determinism and perfInside the
filteredPolls.sort(...)callback,const now = new Date();is evaluated on every comparison. In practice this is fine, but it does extra allocations and can theoretically flipaIsActive/bIsActivemid‑sort if deadlines are very close.Consider hoisting
now:const now = new Date(); const sortedPolls = filteredPolls.sort((a, b) => { const aIsActive = !a.deadline || new Date(a.deadline) > now; const bIsActive = !b.deadline || new Date(b.deadline) > now; // ... });This keeps comparisons cheaper and time‑consistent during a single sort call.
platforms/eVoting/src/app/(app)/page.tsx (1)
20-31: Client sort‑state persistence is sound; consider validating stored sortDirectionThe lazy
useStateinitializers withtypeof window !== "undefined"guards and the write‑backs inhandleSortcorrectly persist sort preferences and are safe for Next.js client components.One small hardening you might consider:
localStorage.getItem("evoting_sortDirection")is blindly cast to"asc" | "desc". If some other script/user sets an unexpected value, it will flow into state. A tiny runtime guard would keep things robust:const stored = localStorage.getItem("evoting_sortDirection"); return stored === "asc" || stored === "desc" ? stored : "asc";Same idea could be applied to
sortFieldif you expect external tampering.Also applies to: 86-101
platforms/eVoting/src/app/(app)/[id]/page.tsx (2)
407-437: Zero‑vote results still visually highlight all options as winnersYou now correctly gate the “Tied”/“Winner” badges on
blindVoteResults.totalVotes > 0, butisWinneris still computed as:const isWinner = result.voteCount === Math.max(...blindVoteResults.optionResults.map(r => r.voteCount));When
totalVotesis 0, everyvoteCountis 0, so every row gets winner styling (green card) even though badges are hidden. If you want a fully neutral look for no‑vote polls, consider tyingisWinnerto the same condition:const hasVotes = blindVoteResults.totalVotes > 0; const maxVotes = Math.max(...blindVoteResults.optionResults.map(r => r.voteCount)); const isWinner = hasVotes && result.voteCount === maxVotes;Same pattern could be applied to the public‑results path where
isWinneris used withresultsData.totalVotes.Also applies to: 424-433
469-487: Ereputation points summary & “Points” label align with backend; just watch numeric formattingThe new turnout suffix:
(resultsData.mode === "ereputation" || selectedPoll?.votingWeight === "ereputation") && resultsData.pointsVoted !== undefined && resultsData.totalEligiblePoints !== undefinedis consistent with how
VoteService.getPollResultsnow sets:
mode: "ereputation"for weighted normal and point polls.pointsVotedas the weighted total.totalEligiblePointsas the sum of reputation scores (×100 for point mode).Similarly, in the non‑points branch:
displayValue = resultsData.mode === "ereputation" ? `${voteCount} Points` : `${voteCount} votes`;correctly re‑labels weighted tallies as “Points” to avoid calling fractional weights “votes”.
If you expect fractional weights (e.g. non‑integer scores), you might want to apply
toFixed/rounding here as you do for percentages, otherwise long decimals could leak into the UI:const formatted = Number.isInteger(voteCount) ? voteCount : voteCount.toFixed(2); displayValue = resultsData.mode === "ereputation" ? `${formatted} Points` : `${formatted} votes`;Purely a cosmetic refinement; current logic is functionally correct.
Also applies to: 483-485, 524-533, 528-529, 548-557
platforms/evoting-api/src/services/VoteService.ts (1)
334-375: Fix field naming to align across all modes: renametotalWeightedPointstototalWeightedVotesThe verification confirms the review comment is valid. The frontend
PollResultsinterface defines onlytotalWeightedVotes?: number, nottotalWeightedPoints. However, both point-mode branches (weighted at line 88 and non-weighted at line 123) returntotalWeightedPoints, which diverges from the normal mode'stotalWeightedVotesand misaligns with the frontend type definition.While this doesn't cause a TypeScript error (the interface field is optional and structural typing allows extra properties), it creates unnecessary confusion and inconsistency across voting modes. Rename both occurrences of
totalWeightedPointstototalWeightedVotesin the point-mode return objects (lines 88 and 123 context):// Line 88 context (weighted points): totalWeightedVotes: totalWeightedPoints, // Rename from totalWeightedPoints // Line 123 context (non-weighted points): totalWeightedVotes: totalPoints, // Rename from totalWeightedPoints: totalPointsThis aligns all modes with the frontend type definition and maintains consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
platforms/eVoting/src/app/(app)/[id]/page.tsx(4 hunks)platforms/eVoting/src/app/(app)/page.tsx(2 hunks)platforms/eVoting/src/lib/pollApi.ts(1 hunks)platforms/evoting-api/src/services/PollService.ts(1 hunks)platforms/evoting-api/src/services/VoteService.ts(3 hunks)
🔇 Additional comments (1)
platforms/eVoting/src/lib/pollApi.ts (1)
114-126: PollResults extension looks consistent with backend shapeAdding
pointsVoted?andtotalEligiblePoints?as optional fields onPollResultsaligns with the new data returned byVoteService.getPollResultsand preserves backwards compatibility for existing callers that ignore them. No issues from a typing or evolution standpoint.
Description of change
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.