Upload ballots refactor - #1611
ArendPeter wants to merge 13 commits into
Conversation
) Task: Issue Equal-Vote#1606 (docs-only ADR for the dual-gate design from PRD Equal-Vote#1603) PRD: Equal-Vote#1603 (Allow election admins to bulk upload ballots) Decisions recorded: - Two independent checks are required to upload a ballot: the existing `canUploadBallots` role permission (who may upload) and the new `allowed_submit_types` election setting (does this election allow it). - Merging the two into one check conflates identity with policy and creates unsolvable edge cases in both directions (all-or-nothing vs. no role gate). - Each gate can evolve independently — role permission can be delegated; setting can gain new channel types — without coupling the two. Files changed: - dev-docs/adr/0003-dual-gate-submission-channel-design.md (new) - CONTEXT.md (added Submission Channel, Paper Ballots, Ballot Source terms) Notes: - No code changes; this is docs-only per the issue's acceptance criteria. - Ballot Source clarification was added to CONTEXT.md to prevent future confusion between the two orthogonal concepts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ual-Vote#1605) Task: Issue 1605 — extract computeRaceOrder, encodeBallotRow, uploadBallotsBatched from UploadElections.tsx into a standalone unit-tested module. Key decisions: - New module: packages/frontend/src/components/Election/Upload/uploadUtils.ts - computeRaceOrder derived from election.races (not from first ballot's votes) - encodeBallotRow looks up each race by race_id, not positional index; races absent from a ballot encode as all-undecided (all null marks) - uploadBallotsBatched fixes silent-discard bug: returns aborted:true + a synthetic failure entry for untried remainder when batch floor is hit - Added Jest to frontend (jest.config.cjs + tsconfig.test.json); test files excluded from main tsconfig to keep tsc build clean Files changed: - packages/frontend/src/components/Election/Upload/uploadUtils.ts (new) - packages/frontend/src/components/Election/Upload/uploadUtils.test.ts (new, 9 tests) - packages/frontend/src/components/Election/Upload/UploadElections.tsx (rewired) - packages/frontend/jest.config.cjs (new) - packages/frontend/tsconfig.test.json (new) - packages/frontend/tsconfig.json (exclude test files) - packages/frontend/package.json (add test script) - package-lock.json (updated) No blockers. All 9 frontend tests + 211 backend tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Drop the UploadBatchedOptions interface since no caller overrides the batch-size defaults; inline them as module constants. - Push into `responses` instead of rebuilding it every batch. - Remove the aborted-branch synthetic error entry that the caller never reads (it only checks `aborted`). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ual-Vote#1604, PRD Equal-Vote#1603) Key decisions: - BallotSubmitType and BallotActionType moved from castVoteController.ts into shared Ballot.ts; BallotAction.action_type retypes from string to BallotActionType - NewBallot gains election_id, status, date_submitted as optional fields - ElectionSettings gains optional allowed_submit_types?: BallotSubmitType[] with DEFAULT_ALLOWED_SUBMIT_TYPES = ['submitted_via_browser', 'submitted_via_discord'] - electionSettingsValidation rejects an explicitly empty allowed_submit_types array - makeBallotEvent enforces allowed_submit_types inside the existing draft/prior_election bypass block, throwing BadRequest 400 naming the disallowed type - makeDevElections.ts typed action_type against BallotActionType (closing the drift gap) - 11 backend tests cover: settings validation, admin accepted/rejected per setting, default-array rejection, draft bypass, and compose with already-voted check Files changed: - packages/shared/src/domain_model/Ballot.ts - packages/shared/src/domain_model/ElectionSettings.ts - packages/backend/src/Controllers/Ballot/castVoteController.ts - packages/backend/src/DevElections/makeDevElections.ts - packages/backend/src/test/TestHelper.ts (added uploadBallots helper) - packages/backend/src/test/allowedSubmitTypes.test.ts (new, 11 tests) Notes: - Upload endpoint returns per-ballot success/failure (HTTP 200 with success:false for rejected ballots), consistent with existing bulk-upload behavior - Frontend enforcement (hiding Vote button, Settings UI checkboxes) is out of scope for this issue per the spec Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Extract BALLOT_SUBMIT_TYPES as a const array in Ballot.ts so the BallotSubmitType union is derived from a single source of truth - Consume BALLOT_SUBMIT_TYPES in electionSettingsValidation instead of re-declaring the list locally - Fix dead code in the empty-array check (the `?? DEFAULT_ALLOWED_SUBMIT_TYPES` fallback was unreachable inside `if (obj.allowed_submit_types !== undefined)`) - Reorder the checks so the empty-array error triggers before value validation Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- 1604: Enforce allowed_submit_types in the ballot-submission pipeline - 1605: Extract shared upload utilities and rewire Upload Elections onto them - 1606: ADR: dual-gate submission-channel design
✅ Deploy Preview for bettervoting ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reachedNext included review available in 49 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
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 |
| @@ -0,0 +1,15 @@ | |||
| module.exports = { | |||
There was a problem hiding this comment.
Adding unit tests to the frontend is a good idea, but I'd use vitest instead of jest
There was a problem hiding this comment.
This looks like it was deleted by accident
| ], | ||
| }, | ||
| ], | ||
| } as unknown as Election; |
There was a problem hiding this comment.
What's going on here 👀
There was a problem hiding this comment.
I see, "as unknown as Election" is used when the original type doesn't quite match Election but it's close enough for the purposes of the test.
| import { NewBallot, RaceCandidateOrder } from '@equal-vote/star-vote-shared/domain_model/Ballot'; | ||
| import { ORDERED_VOTE_TAIL_LENGTH } from '@equal-vote/star-vote-shared/domain_model/OrderedVoteCodec'; | ||
|
|
||
| const mockElection = { |
There was a problem hiding this comment.
NOTE, I think this is fine for now, but for future reference this is somewhat redundant with our library of mocks on the backend. If we proceed with building out the frontend unit testing library, then we could consider sharing a library via the shared package, or defining a separate library for frontend
| // Both races should be all-undecided | ||
| const race1Vote = result.orderedVotes[0]; | ||
| expect(race1Vote[0]).toBeNull(); | ||
| expect(race1Vote[1]).toBeNull(); |
There was a problem hiding this comment.
Shouldn't this also be validating race2Vote?
| }); | ||
|
|
||
| // A simple open election with one STAR race and two candidates. | ||
| const makeElection = (allowed_submit_types?: string[]): Election => ({ |
There was a problem hiding this comment.
Consider using testInputs for this. You could add a new election, but it's even better if you can reuse one
| // orderedVote = [aliceScore, bobScore, overvote_rank, has_duplicate_rank] | ||
| const ORDERED_BALLOT = { orderedVotes: [[5, 0, null, null]] }; | ||
|
|
||
| describe("electionSettingsValidation — allowed_submit_types", () => { |
There was a problem hiding this comment.
Can we move electionSettingsValidation tests to the shared pacakge?
- Restore package-lock.json libc metadata accidentally stripped by the branch - Document why BallotActionType currently mirrors BallotSubmitType - Fix encodeBallotRow test to also assert race2Vote when all races are missing - Reuse testInputs.Election1 as the base for allowedSubmitTypes.test's makeElection - Move electionSettingsValidation unit tests from the backend to the shared package, next to the function they test, adding vitest as its test runner - Switch frontend unit tests from jest to vitest, since frontend never declared its own jest/ts-jest dependency (it only worked via hoisting from backend's devDependencies) - Fix lint-staged failures uncovered while touching these files: unused imports/empty interface in Ballot.ts, and a require()-style dotenv import in allowedSubmitTypes.test.ts Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A2V3GyzesaLkqYxrYLF4cr
Removed test for DEFAULT_ALLOWED_SUBMIT_TYPES.
The root "test" script only ever ran the backend suite (cd packages/backend && npm test), so CI (npm test in node.js.yml) and the pre-push hook (which runs the same command) never executed the frontend or shared tests, even after this branch added real ones. Switch to `npm test -ws --if-present` so it runs test in every workspace that defines one, in workspace order (shared, backend, frontend) — same entry points, now covering all three packages. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A2V3GyzesaLkqYxrYLF4cr
Rebasing onto the removal of the DEFAULT_ALLOWED_SUBMIT_TYPES test left the import unused, which fails lint. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A2V3GyzesaLkqYxrYLF4cr
Cleans up eslint errors (no-explicit-any, no-unused-vars, no-require-imports, prefer-const, no-wrapper-object-types, no-empty-object-type) in files this branch modifies, since lint:diff lints whole changed files rather than just diff hunks. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QWJJYaaaBTSMwGvfpAQwh7
Description
This change begins refactoring the frontend in preparation for a upload ballots feature. It particular it adds an allowed_submit_types so that we can gate different voting submissions (web vs admin upload vs discord), and starts to move the uploadElection functions into a shared utility.