feat: implement poll status management with start and end functionality#906
feat: implement poll status management with start and end functionality#906
Conversation
📝 WalkthroughWalkthroughAdds a poll lifecycle (status: draft|active|ended) with DB migration, APIs and UI to manually start/end polls; enforces status guards to block voting on non-active polls; updates poll creation/deadline behavior and seed script to auto-enroll non-seed users. Changes
Sequence DiagramsequenceDiagram
autonumber
participant Creator as Creator
participant Client as Frontend
participant API as PollController (API)
participant Service as PollService
participant DB as Database
participant Voter as Voter
Creator->>Client: Click "Start Vote"
Client->>API: POST /api/polls/:id/start (userId)
API->>Service: startPoll(id, userId)
Service->>DB: verify ownership & poll.status == "draft"
Service->>DB: update polls.status = "active"
Service-->>API: updated Poll
API-->>Client: 200 OK + Poll(status: active)
Note over Voter: Voting is allowed now
Voter->>Client: submit vote
Client->>API: POST /api/votes (pollId,...)
API->>Service: createVote(...)
Service->>DB: check poll.status == "active"
Service->>DB: insert vote
Service-->>API: vote created
API-->>Client: 200 OK
Creator->>Client: Click "End Vote"
Client->>API: POST /api/polls/:id/end (userId)
API->>Service: endPoll(id, userId)
Service->>DB: verify ownership & poll.status == "active"
Service->>DB: update polls.status = "ended"
Service-->>API: updated Poll + results link
API-->>Client: 200 OK + Poll(status: ended)
Note over Voter: Voting is blocked now
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
platforms/evoting/api/src/services/DeadlineCheckService.ts (1)
28-35:⚠️ Potential issue | 🔴 CriticalMissing
status = 'active'filter allows draft polls to be auto-ended.The query finds all polls with expired deadlines but does not filter by
status. According to the relevant code snippets,PollService.endPollvalidates that only "active" polls can be ended. However, this scheduler bypasses that check, potentially auto-ending "draft" polls that were never manually started.This creates an inconsistency: a poll created with a past deadline (or a deadline that passes before the creator starts it) would be marked as "ended" without ever being "active".
🐛 Proposed fix
const expiredPolls = await this.pollRepository .createQueryBuilder("poll") .leftJoinAndSelect("poll.creator", "creator") .where("poll.deadline < :now", { now }) .andWhere("poll.deadlineMessageSent = :sent", { sent: false }) .andWhere("poll.deadline IS NOT NULL") + .andWhere("poll.status = :status", { status: "active" }) .getMany();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/evoting/api/src/services/DeadlineCheckService.ts` around lines 28 - 35, The query in DeadlineCheckService that builds expiredPolls misses filtering by poll status, so draft polls with past deadlines may be processed; update the createQueryBuilder call on this.pollRepository to include a condition filtering for only active polls (e.g., .andWhere("poll.status = :status", { status: "active" }) or equivalent) so DeadlineCheckService only selects polls that PollService.endPoll is allowed to end.platforms/evoting/client/src/app/(app)/page.tsx (1)
238-240:⚠️ Potential issue | 🟡 MinorInconsistent deadline display between mobile and desktop views.
The mobile view shows "No deadline" (line 239) while the desktop view shows "Manual" (line 346) for polls without a deadline. This should be consistent.
🔧 Proposed fix
<div className="text-xs text-gray-500"> - {poll.deadline ? `Deadline: ${new Date(poll.deadline).toLocaleDateString()}` : "No deadline"} + {poll.deadline ? `Deadline: ${new Date(poll.deadline).toLocaleDateString()}` : "Manual"} </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/evoting/client/src/app/`(app)/page.tsx around lines 238 - 240, The mobile deadline display uses {poll.deadline ? `Deadline: ${new Date(poll.deadline).toLocaleDateString()}` : "No deadline"} while the desktop view renders "Manual" for missing deadlines; make them consistent by choosing one label and updating both JSX spots (the mobile JSX that checks poll.deadline and the desktop JSX that currently outputs "Manual") to use the same fallback string (e.g., "No deadline" or "Manual") so both mobile and desktop show identical text for polls without a deadline.
🧹 Nitpick comments (2)
platforms/evoting/api/scripts/seed.ts (1)
501-512: Consider batching group saves to improve performance.The current implementation calls
groupRepo.save(group)inside a nested loop, which results in O(users × groups) database writes. For development seeding this is acceptable, but if non-seed user counts grow, consider batching updates per group.♻️ Suggested optimization
for (const user of nonSeedUsers) { for (const group of groupsToUpdate) { const isAlreadyMember = group.members.some((m) => m.id === user.id); if (!isAlreadyMember) { group.members.push(user); group.participants.push(user); - await groupRepo.save(group); console.log(` ✅ Added "${user.name || user.ename}" to "${group.name}"`); } } } + // Save all groups after modifications + await groupRepo.save(groupsToUpdate);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/evoting/api/scripts/seed.ts` around lines 501 - 512, The nested loop over nonSeedUsers and groupsToUpdate causes repeated writes via groupRepo.save(group); instead batch updates by iterating groupsToUpdate once, for each group merge all new users from nonSeedUsers into group.members and group.participants (check with the same isAlreadyMember predicate), then call groupRepo.save(...) once per group (or use groupRepo.saveMany/saveAll if available) to reduce O(users×groups) DB writes; update the code paths around nonSeedUsers, groupsToUpdate, group.members, group.participants, and groupRepo.save to implement this batching.platforms/evoting/api/src/services/PollService.ts (1)
306-335: Consider adding error handling for the final fetch.The
startPollmethod correctly validates ownership and status before transitioning. However, line 334 uses a non-null assertion after fetching the updated poll. While unlikely, if the update succeeds but the subsequent fetch fails, this would throw an unhandled error.♻️ Suggested improvement
await this.pollRepository.update(id, { status: "active" }); // Send system message that voting is now open if (poll.groupId) { await this.messageService.createVoteCreatedMessage( poll.groupId, poll.title, poll.id, poll.creator.name, poll.deadline ); } - return (await this.getPollById(id))!; + const updatedPoll = await this.getPollById(id); + if (!updatedPoll) { + throw new Error("Failed to retrieve updated poll"); + } + return updatedPoll; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/evoting/api/src/services/PollService.ts` around lines 306 - 335, The final fetch in PollService.startPoll uses a non-null assertion on (await this.getPollById(id))!, which can throw if the fetch fails; replace this with explicit error handling: call const updated = await this.getPollById(id), check if updated is null/undefined and throw a clear Error like "Failed to fetch updated poll after start" (or return the poll object returned by this.pollRepository.update if it returns the updated entity), and consider wrapping the post-update notification and fetch in try/catch to log or rethrow a meaningful error; reference startPoll, getPollById, and pollRepository.update when applying the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/evoting/api/src/controllers/PollController.ts`:
- Around line 188-194: The error handling in endPoll (PollController.endPoll)
returns HTTP 400 for authorization-related failures; update the handler so that
when error.message matches "Not authorized to end this poll" (and any other
auth-related message like "Only active polls can be ended" if intended to be an
auth check) the response uses res.status(403).json(...) instead of
res.status(400).json(...); locate the branch that checks error instanceof Error
and change the status code for the authorization failure condition(s)
accordingly.
- Around line 168-174: Update the error handling in PollController.ts to return
403 Forbidden for authorization failures: when error.message === "Not authorized
to start this poll" change the response to res.status(403).json({ error:
error.message }) while keeping "Poll not found" as 404 and "Only draft polls can
be started" as 400; locate this logic in the error instanceof Error block that
currently checks those error.message strings and adjust the res.status for the
authorization case accordingly.
In `@platforms/evoting/client/src/app/`(app)/[id]/page.tsx:
- Around line 170-186: The effect that computes timeRemaining in the component
uses selectedPoll?.status but the dependency array only includes
selectedPoll?.deadline and pollExists; update the useEffect dependencies to
include selectedPoll?.status so the effect re-runs when the poll status changes
(target the useEffect that calls setTimeRemaining and references
selectedPoll?.deadline and selectedPoll?.status to locate the spot).
---
Outside diff comments:
In `@platforms/evoting/api/src/services/DeadlineCheckService.ts`:
- Around line 28-35: The query in DeadlineCheckService that builds expiredPolls
misses filtering by poll status, so draft polls with past deadlines may be
processed; update the createQueryBuilder call on this.pollRepository to include
a condition filtering for only active polls (e.g., .andWhere("poll.status =
:status", { status: "active" }) or equivalent) so DeadlineCheckService only
selects polls that PollService.endPoll is allowed to end.
In `@platforms/evoting/client/src/app/`(app)/page.tsx:
- Around line 238-240: The mobile deadline display uses {poll.deadline ?
`Deadline: ${new Date(poll.deadline).toLocaleDateString()}` : "No deadline"}
while the desktop view renders "Manual" for missing deadlines; make them
consistent by choosing one label and updating both JSX spots (the mobile JSX
that checks poll.deadline and the desktop JSX that currently outputs "Manual")
to use the same fallback string (e.g., "No deadline" or "Manual") so both mobile
and desktop show identical text for polls without a deadline.
---
Nitpick comments:
In `@platforms/evoting/api/scripts/seed.ts`:
- Around line 501-512: The nested loop over nonSeedUsers and groupsToUpdate
causes repeated writes via groupRepo.save(group); instead batch updates by
iterating groupsToUpdate once, for each group merge all new users from
nonSeedUsers into group.members and group.participants (check with the same
isAlreadyMember predicate), then call groupRepo.save(...) once per group (or use
groupRepo.saveMany/saveAll if available) to reduce O(users×groups) DB writes;
update the code paths around nonSeedUsers, groupsToUpdate, group.members,
group.participants, and groupRepo.save to implement this batching.
In `@platforms/evoting/api/src/services/PollService.ts`:
- Around line 306-335: The final fetch in PollService.startPoll uses a non-null
assertion on (await this.getPollById(id))!, which can throw if the fetch fails;
replace this with explicit error handling: call const updated = await
this.getPollById(id), check if updated is null/undefined and throw a clear Error
like "Failed to fetch updated poll after start" (or return the poll object
returned by this.pollRepository.update if it returns the updated entity), and
consider wrapping the post-update notification and fetch in try/catch to log or
rethrow a meaningful error; reference startPoll, getPollById, and
pollRepository.update when applying the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 206d9102-e5e3-486e-aade-f63c25cfc930
📒 Files selected for processing (12)
platforms/evoting/api/scripts/seed.tsplatforms/evoting/api/src/controllers/PollController.tsplatforms/evoting/api/src/database/entities/Poll.tsplatforms/evoting/api/src/database/migrations/1772851200000-migration.tsplatforms/evoting/api/src/index.tsplatforms/evoting/api/src/services/DeadlineCheckService.tsplatforms/evoting/api/src/services/PollService.tsplatforms/evoting/api/src/services/VoteService.tsplatforms/evoting/client/src/app/(app)/[id]/page.tsxplatforms/evoting/client/src/app/(app)/create/page.tsxplatforms/evoting/client/src/app/(app)/page.tsxplatforms/evoting/client/src/lib/pollApi.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
platforms/evoting/client/src/app/(app)/[id]/page.tsx (3)
553-574: Consider adding confirmation for the End Vote action.Ending a poll is a destructive, irreversible action. The "End Vote" button executes immediately without confirmation. Consider adding a confirmation dialog to prevent accidental poll termination.
💡 Example using window.confirm (or a modal for better UX)
- {isCreator && selectedPoll.status === "active" && !selectedPoll.deadline && ( + {isCreator && selectedPoll.status === "active" && !selectedPoll.deadline && ( <div className="mt-4 flex justify-center"> <Button - onClick={handleEndPoll} + onClick={() => { + if (window.confirm("Are you sure you want to end this vote? This action cannot be undone.")) { + handleEndPoll(); + } + }} className="bg-red-600 hover:bg-red-700 text-white" > <Square className="w-4 h-4 mr-2" /> End Vote </Button> </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/evoting/client/src/app/`(app)/[id]/page.tsx around lines 553 - 574, The End Vote button currently calls handleEndPoll directly (rendered inside the Button when isCreator && selectedPoll.status === "active" && !selectedPoll.deadline), so add a confirmation step before invoking handleEndPoll: update the Button's onClick handler to prompt the user (e.g., use window.confirm or a modal) and only call handleEndPoll if the user confirms, ensuring the confirmation text clearly warns that ending the poll is irreversible; keep the same Button and handler names (Button, handleEndPoll, selectedPoll) so the change is localized and easy to review.
332-361: Add loading state to prevent duplicate submissions.The
handleStartPollandhandleEndPollhandlers lack loading state protection. A user could double-click the button and trigger multiple API calls before the UI updates, potentially causing race conditions or duplicate requests.♻️ Suggested fix
+ const [isPollActionLoading, setIsPollActionLoading] = useState(false); + const handleStartPoll = async () => { - if (!pollId) return; + if (!pollId || isPollActionLoading) return; + setIsPollActionLoading(true); try { const updated = await pollApi.startPoll(pollId); setSelectedPoll(updated); toast({ title: "Vote Started", description: "Voting is now open." }); } catch (error: any) { toast({ title: "Error", description: error?.response?.data?.error || "Failed to start poll.", variant: "destructive", }); + } finally { + setIsPollActionLoading(false); } }; const handleEndPoll = async () => { - if (!pollId) return; + if (!pollId || isPollActionLoading) return; + setIsPollActionLoading(true); try { const updated = await pollApi.endPoll(pollId); setSelectedPoll(updated); await fetchVoteData(); toast({ title: "Vote Ended", description: "Voting has been closed. Results are now available." }); } catch (error: any) { toast({ title: "Error", description: error?.response?.data?.error || "Failed to end poll.", variant: "destructive", }); + } finally { + setIsPollActionLoading(false); } };Then use
isPollActionLoadingto disable the buttons.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/evoting/client/src/app/`(app)/[id]/page.tsx around lines 332 - 361, The handlers handleStartPoll and handleEndPoll currently allow duplicate submissions; add a boolean loading state (e.g., isPollActionLoading) in the component, guard each handler to return early if isPollActionLoading is true, set isPollActionLoading = true at the start of the try block and set it back to false in finally, and update the poll action buttons to be disabled when isPollActionLoading is true so users cannot double-click and trigger concurrent API calls.
904-906: Consider extracting repeated rank data normalization into a helper.The pattern for handling both rank data formats (
{ "0": 1 }and{ "ranks": { "0": 1 } }) is repeated ~8 times throughout this file. Extracting this into a utility function would improve maintainability and reduce the risk of inconsistent handling.💡 Suggested helper function
// Add near the top of the component or extract to a utility file const normalizeRankData = (rawRankData: any): Record<string, number> | null => { if (!rawRankData) return null; if (rawRankData.ranks && typeof rawRankData.ranks === 'object') { return rawRankData.ranks; } return rawRankData; };Then replace all instances like:
-const rankData = rawRankData?.ranks && typeof rawRankData.ranks === 'object' - ? rawRankData.ranks - : rawRankData; +const rankData = normalizeRankData(rawRankData);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/evoting/client/src/app/`(app)/[id]/page.tsx around lines 904 - 906, Extract the repeated rank normalization into a helper named normalizeRankData and replace each occurrence of the inline pattern that computes rankData (e.g., the block assigning const rankData = rawRankData?.ranks && typeof rawRankData.ranks === 'object' ? rawRankData.ranks : rawRankData;) with a call to normalizeRankData(rawRankData); implement normalizeRankData to return null for falsy input, return rawRankData.ranks when it exists and is an object, otherwise return rawRankData; update all ~8 places (instances referencing rankData/rawRankData) to use this helper to ensure consistent handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/evoting/client/src/app/`(app)/[id]/page.tsx:
- Around line 1618-1622: The ordinal suffix logic in the return expression that
builds the string using rank (see variables rawRankData, rankData and rank)
incorrectly hardcodes '2nd' causing outputs like "22nd"; change the suffix
branch from '2nd' to 'nd' so the template `${rank}${...}` produces "2nd"
correctly (i.e., replace the `rank === 2 ? '2nd'` case with `rank === 2 ?
'nd'`). Optionally, consider refactoring this logic into a small helper (e.g.,
getOrdinalSuffix(rank)) for clarity and to handle edge cases like 11–13
consistently.
---
Nitpick comments:
In `@platforms/evoting/client/src/app/`(app)/[id]/page.tsx:
- Around line 553-574: The End Vote button currently calls handleEndPoll
directly (rendered inside the Button when isCreator && selectedPoll.status ===
"active" && !selectedPoll.deadline), so add a confirmation step before invoking
handleEndPoll: update the Button's onClick handler to prompt the user (e.g., use
window.confirm or a modal) and only call handleEndPoll if the user confirms,
ensuring the confirmation text clearly warns that ending the poll is
irreversible; keep the same Button and handler names (Button, handleEndPoll,
selectedPoll) so the change is localized and easy to review.
- Around line 332-361: The handlers handleStartPoll and handleEndPoll currently
allow duplicate submissions; add a boolean loading state (e.g.,
isPollActionLoading) in the component, guard each handler to return early if
isPollActionLoading is true, set isPollActionLoading = true at the start of the
try block and set it back to false in finally, and update the poll action
buttons to be disabled when isPollActionLoading is true so users cannot
double-click and trigger concurrent API calls.
- Around line 904-906: Extract the repeated rank normalization into a helper
named normalizeRankData and replace each occurrence of the inline pattern that
computes rankData (e.g., the block assigning const rankData = rawRankData?.ranks
&& typeof rawRankData.ranks === 'object' ? rawRankData.ranks : rawRankData;)
with a call to normalizeRankData(rawRankData); implement normalizeRankData to
return null for falsy input, return rawRankData.ranks when it exists and is an
object, otherwise return rawRankData; update all ~8 places (instances
referencing rankData/rawRankData) to use this helper to ensure consistent
handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9eb7e3b8-5853-4d66-9ba3-c669f04fec77
📒 Files selected for processing (2)
platforms/evoting/api/src/controllers/PollController.tsplatforms/evoting/client/src/app/(app)/[id]/page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- platforms/evoting/api/src/controllers/PollController.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
platforms/evoting/client/src/app/(app)/[id]/page.tsx (1)
332-361: Consider adding loading states to prevent double-click issues.The
handleStartPollandhandleEndPollhandlers don't have loading indicators. Users could click multiple times before the API responds, potentially causing duplicate requests or confusing UX.♻️ Suggested improvement
+ const [isStartingPoll, setIsStartingPoll] = useState(false); + const [isEndingPoll, setIsEndingPoll] = useState(false); + const handleStartPoll = async () => { if (!pollId) return; + setIsStartingPoll(true); try { const updated = await pollApi.startPoll(pollId); setSelectedPoll(updated); toast({ title: "Vote Started", description: "Voting is now open." }); } catch (error: any) { toast({ title: "Error", description: error?.response?.data?.error || "Failed to start poll.", variant: "destructive", }); + } finally { + setIsStartingPoll(false); } }; const handleEndPoll = async () => { if (!pollId) return; + setIsEndingPoll(true); try { const updated = await pollApi.endPoll(pollId); setSelectedPoll(updated); await fetchVoteData(); toast({ title: "Vote Ended", description: "Voting has been closed. Results are now available." }); } catch (error: any) { toast({ title: "Error", description: error?.response?.data?.error || "Failed to end poll.", variant: "destructive", }); + } finally { + setIsEndingPoll(false); } };Then use
disabled={isStartingPoll}anddisabled={isEndingPoll}on the respective buttons at lines 555-561 and 566-572.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/evoting/client/src/app/`(app)/[id]/page.tsx around lines 332 - 361, Add local loading state flags (e.g., isStartingPoll, setIsStartingPoll and isEndingPoll, setIsEndingPoll) and use them in handleStartPoll and handleEndPoll: set the appropriate flag to true before calling pollApi.startPoll/pollApi.endPoll, run the API call in try/catch, then clear the flag in a finally block (or after both success and error paths); update setSelectedPoll and fetchVoteData as before. Also disable the corresponding Start and End buttons using disabled={isStartingPoll} and disabled={isEndingPoll} (and optionally show a spinner when the flag is true) so repeated clicks are prevented while the request is in-flight.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@platforms/evoting/client/src/app/`(app)/[id]/page.tsx:
- Around line 332-361: Add local loading state flags (e.g., isStartingPoll,
setIsStartingPoll and isEndingPoll, setIsEndingPoll) and use them in
handleStartPoll and handleEndPoll: set the appropriate flag to true before
calling pollApi.startPoll/pollApi.endPoll, run the API call in try/catch, then
clear the flag in a finally block (or after both success and error paths);
update setSelectedPoll and fetchVoteData as before. Also disable the
corresponding Start and End buttons using disabled={isStartingPoll} and
disabled={isEndingPoll} (and optionally show a spinner when the flag is true) so
repeated clicks are prevented while the request is in-flight.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5123da19-2836-44a1-8b1d-7d1291ed7fe6
📒 Files selected for processing (1)
platforms/evoting/client/src/app/(app)/[id]/page.tsx
Description of change
API:
statusfield (draft|active|ended) to thePollentity with a database migrationstartPollandendPollendpoints (POST /api/polls/:id/start,POST /api/polls/:id/end)draft; polls with a deadline start asactivedraftorendedpolls (regular, delegated, and blind votes)DeadlineCheckServicenow setsstatus: "ended"when a deadline expiresClient:
status === "active"; draft state shows a "Voting Has Not Started" placeholderIssue Number
Closes #869
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Bug Fixes