-
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
Update VxAdmin to support importing new CVR export format #3956
Changes from 9 commits
05c37b9
f8abdf5
8da64a4
85674ce
c3ae016
6ea6ad4
9474952
830a757
a668787
84480ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,283 @@ | ||
/* c8 ignore start */ | ||
import * as fs from 'fs/promises'; | ||
import { sha256 } from 'js-sha256'; | ||
import path from 'path'; | ||
import { v4 as uuid } from 'uuid'; | ||
import { isTestReport, readCastVoteRecordExport } from '@votingworks/backend'; | ||
import { assert, assertDefined, err, ok, Result } from '@votingworks/basics'; | ||
import { | ||
BallotId, | ||
BallotPageLayoutSchema, | ||
CVR, | ||
ElectionDefinition, | ||
getBallotStyle, | ||
getContests, | ||
getPrecinctById, | ||
safeParseJson, | ||
} from '@votingworks/types'; | ||
import { | ||
BooleanEnvironmentVariableName, | ||
castVoteRecordHasValidContestReferences, | ||
convertCastVoteRecordVotesToTabulationVotes, | ||
isFeatureFlagEnabled, | ||
} from '@votingworks/utils'; | ||
|
||
import { Store } from './store'; | ||
import { | ||
CastVoteRecordElectionDefinitionValidationError, | ||
CvrFileImportInfo, | ||
CvrFileMode, | ||
ImportCastVoteRecordsError, | ||
} from './types'; | ||
|
||
/** | ||
* Validates that the fields in a cast vote record and the election definition correspond | ||
*/ | ||
function validateCastVoteRecordAgainstElectionDefinition( | ||
castVoteRecord: CVR.CVR, | ||
electionDefinition: ElectionDefinition | ||
): Result<void, CastVoteRecordElectionDefinitionValidationError> { | ||
function wrapError( | ||
error: Omit<CastVoteRecordElectionDefinitionValidationError, 'type'> | ||
): Result<void, CastVoteRecordElectionDefinitionValidationError> { | ||
return err({ ...error, type: 'invalid-cast-vote-record' }); | ||
} | ||
|
||
const { election, electionHash } = electionDefinition; | ||
|
||
if ( | ||
castVoteRecord.ElectionId !== electionHash && | ||
!isFeatureFlagEnabled( | ||
BooleanEnvironmentVariableName.SKIP_CVR_ELECTION_HASH_CHECK | ||
) | ||
) { | ||
return wrapError({ subType: 'election-mismatch' }); | ||
} | ||
|
||
const precinct = getPrecinctById({ | ||
election, | ||
precinctId: castVoteRecord.BallotStyleUnitId, | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you wanted, you could use the cached versions of these functions from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! 84480ad There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Haters always gonna hate on the caching but we've got lots of data to process no time for haters. |
||
if (!precinct) { | ||
return wrapError({ subType: 'precinct-not-found' }); | ||
} | ||
|
||
const ballotStyle = getBallotStyle({ | ||
ballotStyleId: castVoteRecord.BallotStyleId, | ||
election, | ||
}); | ||
if (!ballotStyle) { | ||
return wrapError({ subType: 'ballot-style-not-found' }); | ||
} | ||
|
||
const contestValidationResult = castVoteRecordHasValidContestReferences( | ||
castVoteRecord, | ||
getContests({ ballotStyle, election }) | ||
); | ||
if (contestValidationResult.isErr()) { | ||
return wrapError({ subType: contestValidationResult.err() }); | ||
} | ||
|
||
return ok(); | ||
} | ||
|
||
/** | ||
* Imports cast vote records given a cast vote record export directory path | ||
*/ | ||
export async function importCastVoteRecords( | ||
store: Store, | ||
exportDirectoryPath: string | ||
): Promise<Result<CvrFileImportInfo, ImportCastVoteRecordsError>> { | ||
const electionId = assertDefined(store.getCurrentElectionId()); | ||
const { electionDefinition } = assertDefined(store.getElection(electionId)); | ||
|
||
const readResult = await readCastVoteRecordExport(exportDirectoryPath); | ||
if (readResult.isErr()) { | ||
return readResult; | ||
} | ||
const { castVoteRecordExportMetadata, castVoteRecords } = readResult.ok(); | ||
const { castVoteRecordReportMetadata } = castVoteRecordExportMetadata; | ||
|
||
const exportDirectoryName = path.basename(exportDirectoryPath); | ||
// Hashing the export metadata, which includes a root hash of all the individual cast vote | ||
// records, gives us a complete hash of the entire export | ||
const exportHash = sha256(JSON.stringify(castVoteRecordExportMetadata)); | ||
const exportedTimestamp = castVoteRecordReportMetadata.GeneratedDate; | ||
|
||
// Ensure that the records to be imported match the mode (test vs. official) of previously | ||
// imported records | ||
const mode: CvrFileMode = isTestReport(castVoteRecordReportMetadata) | ||
? 'test' | ||
: 'official'; | ||
const currentMode = store.getCurrentCvrFileModeForElection(electionId); | ||
if (currentMode !== 'unlocked' && mode !== currentMode) { | ||
return err({ type: 'invalid-mode', currentMode }); | ||
} | ||
|
||
const existingImportId = store.getCastVoteRecordFileByHash( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Throughout the code and UI, we talk about a singular CVR file per export, which is no longer the case. I'm going to update terminology in a follow-up PR |
||
electionId, | ||
exportHash | ||
); | ||
if (existingImportId) { | ||
return ok({ | ||
id: existingImportId, | ||
alreadyPresent: store.getCastVoteRecordCountByFileId(existingImportId), | ||
exportedTimestamp, | ||
fileMode: mode, | ||
fileName: exportDirectoryName, | ||
newlyAdded: 0, | ||
wasExistingFile: true, | ||
}); | ||
} | ||
|
||
return await store.withTransaction(async () => { | ||
const scannerIds = new Set<string>(); | ||
for (const vxBatch of castVoteRecordReportMetadata.vxBatch) { | ||
store.addScannerBatch({ | ||
batchId: vxBatch['@id'], | ||
electionId, | ||
label: vxBatch.BatchLabel, | ||
scannerId: vxBatch.CreatingDeviceId, | ||
}); | ||
scannerIds.add(vxBatch.CreatingDeviceId); | ||
} | ||
|
||
// Create a top-level record for the import | ||
const importId = uuid(); | ||
store.addCastVoteRecordFileRecord({ | ||
id: importId, | ||
electionId, | ||
exportedTimestamp, | ||
filename: exportDirectoryName, | ||
isTestMode: isTestReport(castVoteRecordReportMetadata), | ||
scannerIds, | ||
sha256Hash: exportHash, | ||
}); | ||
|
||
let castVoteRecordIndex = 0; | ||
let newlyAdded = 0; | ||
let alreadyPresent = 0; | ||
const precinctIds = new Set<string>(); | ||
for await (const castVoteRecordResult of castVoteRecords) { | ||
if (castVoteRecordResult.isErr()) { | ||
return err({ | ||
...castVoteRecordResult.err(), | ||
index: castVoteRecordIndex, | ||
}); | ||
} | ||
const { | ||
castVoteRecord, | ||
castVoteRecordBallotSheetId, | ||
castVoteRecordCurrentSnapshot, | ||
castVoteRecordWriteIns, | ||
referencedFiles, | ||
} = castVoteRecordResult.ok(); | ||
|
||
const validationResult = validateCastVoteRecordAgainstElectionDefinition( | ||
castVoteRecord, | ||
electionDefinition | ||
); | ||
if (validationResult.isErr()) { | ||
return err({ ...validationResult.err(), index: castVoteRecordIndex }); | ||
} | ||
|
||
// Add an individual cast vote record to the import | ||
const votes = convertCastVoteRecordVotesToTabulationVotes( | ||
castVoteRecordCurrentSnapshot | ||
); | ||
const addCastVoteRecordResult = store.addCastVoteRecordFileEntry({ | ||
ballotId: castVoteRecord.UniqueId as BallotId, | ||
cvr: { | ||
ballotStyleId: castVoteRecord.BallotStyleId, | ||
batchId: castVoteRecord.BatchId, | ||
card: castVoteRecordBallotSheetId | ||
? { type: 'hmpb', sheetNumber: castVoteRecordBallotSheetId } | ||
: { type: 'bmd' }, | ||
precinctId: castVoteRecord.BallotStyleUnitId, | ||
votes, | ||
votingMethod: castVoteRecord.vxBallotType, | ||
}, | ||
cvrFileId: importId, | ||
electionId, | ||
}); | ||
if (addCastVoteRecordResult.isErr()) { | ||
return err({ | ||
...addCastVoteRecordResult.err(), | ||
index: castVoteRecordIndex, | ||
}); | ||
} | ||
const { cvrId: castVoteRecordId, isNew: isCastVoteRecordNew } = | ||
addCastVoteRecordResult.ok(); | ||
|
||
if (isCastVoteRecordNew) { | ||
const hmpbCastVoteRecordWriteIns = castVoteRecordWriteIns.filter( | ||
(castVoteRecordWriteIn) => castVoteRecordWriteIn.side | ||
); | ||
if (hmpbCastVoteRecordWriteIns.length > 0) { | ||
// Guaranteed to exist given validation in readCastVoteRecordExport | ||
assert(referencedFiles !== undefined); | ||
|
||
for (const i of [0, 1] as const) { | ||
const imageData = await fs.readFile( | ||
referencedFiles.imageFilePaths[i] | ||
); | ||
const parseLayoutResult = safeParseJson( | ||
await fs.readFile(referencedFiles.layoutFilePaths[i], 'utf8'), | ||
BallotPageLayoutSchema | ||
); | ||
if (parseLayoutResult.isErr()) { | ||
return err({ | ||
type: 'invalid-cast-vote-record', | ||
subType: 'layout-parse-error', | ||
index: castVoteRecordIndex, | ||
}); | ||
} | ||
store.addBallotImage({ | ||
cvrId: castVoteRecordId, | ||
imageData, | ||
pageLayout: parseLayoutResult.ok(), | ||
side: (['front', 'back'] as const)[i], | ||
}); | ||
} | ||
|
||
for (const hmpbCastVoteRecordWriteIn of hmpbCastVoteRecordWriteIns) { | ||
store.addWriteIn({ | ||
castVoteRecordId, | ||
contestId: hmpbCastVoteRecordWriteIn.contestId, | ||
electionId, | ||
optionId: hmpbCastVoteRecordWriteIn.optionId, | ||
side: assertDefined(hmpbCastVoteRecordWriteIn.side), | ||
}); | ||
} | ||
} | ||
} | ||
|
||
if (isCastVoteRecordNew) { | ||
newlyAdded += 1; | ||
} else { | ||
alreadyPresent += 1; | ||
} | ||
precinctIds.add(castVoteRecord.BallotStyleUnitId); | ||
|
||
castVoteRecordIndex += 1; | ||
} | ||
|
||
// TODO: Calculate the precinct list before iterating through cast vote records, once there is | ||
// only one geopolitical unit per batch | ||
store.updateCastVoteRecordFileRecord({ | ||
id: importId, | ||
precinctIds, | ||
}); | ||
Comment on lines
+265
to
+270
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that we have a metadata file, I'm hoping we can eventually get the precinct list in the file up front to avoid this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 Opened an issue for myself so that I don't forget to revisit this: #3968 |
||
|
||
return ok({ | ||
id: importId, | ||
alreadyPresent, | ||
exportedTimestamp, | ||
fileMode: mode, | ||
fileName: exportDirectoryName, | ||
newlyAdded, | ||
wasExistingFile: false, | ||
}); | ||
}); | ||
} | ||
/* c8 ignore stop */ |
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.
How are you handling the manual file selection use case? As in line 446 below.
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.
Ah good catch, I'm not handling that case.
That said, it turns out that manual file selection is already broken because it only allows you to select an individual file, but for quite some time now, we've been exporting a directory and not a file.
Gonna revisit this holistically as its own issue, potentially just removing the manual option altogether, after checking in with others: #3967
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.
It's a bit of a hidden feature that I failed to properly publicize, but manual file selection is not broken. When I switched cast vote records to directories, I added the indicated logic so that you could select the
cast-vote-record-report.json
at the root of the directory, which would import the directory.I use this escape hatch all the time when doing manual testing in VxAdmin. It's far easier than having to always get a cast vote record report on a real or mocked USB drive, so I'd vote strongly to keep the option or something like it.
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.
Ah really great to know! In that case, will plan on maintaining