-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Manual Tally Policy - CSV Tally Reports #4074
Conversation
d7996c7
to
bb5557c
Compare
6dd3f97
to
bf5e3a2
Compare
ad465cc
to
592f43f
Compare
combinedContestResults.tallies[candidateTally.id] = candidateTally; | ||
combinedContestResults.tallies[candidateTally.id] = { | ||
...candidateTally, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙈
export function getTallyReportCandidateRows({ | ||
contest, | ||
scannedContestResults, | ||
manualContestResults, | ||
}: { | ||
contest: CandidateContest; | ||
scannedContestResults: Tabulation.CandidateContestResults; | ||
manualContestResults?: Tabulation.CandidateContestResults; | ||
}): TallyReportCandidateRow[] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that for the use case in this PR, this is a little overbuilt. But. I'm breaking this logic out into a util because I will use the same logic for frontend tally reports, and add different algorithms for grouping / showing write-in candidates. Either "show all", or "show significant." Future features, like the ability to have official write-in candidates, could add more algorithms here. Will build on this in next PR.
export const writeInCandidate: Candidate = { | ||
id: 'write-in', | ||
name: 'Write-In', | ||
isWriteIn: true, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While not strictly necessary, I thought it was better to keep this under tabulation
rather than election
because it really is a tabulation tool. Also, use variable naming to indicate it represents a generic or pending write-in, rather than perhaps thinking it is an actual write-in.
isWriteIn: true, | ||
}; | ||
|
||
export const PENDING_WRITE_IN_ID = GENERIC_WRITE_IN_ID; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am leaving these two with the same IDs because they should never exist in a list together, and changing IDs risked breaking something. But I separated out PENDING
from GENERIC
to give them different default names, and to make code using them more readable.
if (hasManualResults) { | ||
headers.push('Manual Votes', 'Scanned Votes'); | ||
} | ||
headers.push('Total Votes'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's no manual results, we could just keep the column name here as "Votes." But I assume for external systems consuming our .csv files, it will be less confusing if the column names do not change.
review by commit, there's a refactor and bug fix in the log
Overview
Closes #4069 alongside #4073. Updates the tally CSV exports so that they split manual and scanned results.
Demo Video or Screenshot
N/A, example file attached:
TEST-full-election-tally-report__2023-10-23_13-11-49.csv
Testing Plan
Automated testing.
Checklist