Skip to content
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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Sep 13, 2023
Merged

Update VxAdmin to support importing new CVR export format #3956

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
05c37b9
Move VxAdmin CVR helpers to libs/utils
arsalansufi Sep 12, 2023
f8abdf5
Add libs/utils helper for parsing new export directory name format
arsalansufi Sep 12, 2023
8da64a4
Fix artifact authentication bug
arsalansufi Sep 12, 2023
85674ce
Specify encoding when reading files, rather than converting after
arsalansufi Sep 12, 2023
c3ae016
Add libs/backend helpers for importing CVRs
arsalansufi Sep 12, 2023
6ea6ad4
Update logic for listing CVR exports on a USB
arsalansufi Sep 12, 2023
9474952
Add new VxAdmin CVR import function
arsalansufi Sep 12, 2023
830a757
Use new CVR import function in API
arsalansufi Sep 12, 2023
a668787
Add client-side logic for user readable messages
arsalansufi Sep 12, 2023
84480ad
Use optimized helpers in libs/utils
arsalansufi Sep 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion apps/admin/backend/src/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ import { createReadStream, createWriteStream, promises as fs, Stats } from 'fs';
import { basename, dirname, join } from 'path';
import {
BALLOT_PACKAGE_FOLDER,
BooleanEnvironmentVariableName,
CAST_VOTE_RECORD_REPORT_FILENAME,
generateFilenameForBallotExportPackage,
groupMapToGroupList,
isFeatureFlagEnabled,
isIntegrationTest,
parseCastVoteRecordReportDirectoryName,
} from '@votingworks/utils';
Expand All @@ -48,6 +50,7 @@ import {
CvrFileMode,
ElectionRecord,
ExportDataResult,
ImportCastVoteRecordsError,
ManualResultsIdentifier,
ManualResultsMetadataRecord,
ManualResultsRecord,
Expand Down Expand Up @@ -86,6 +89,7 @@ import { getOverallElectionWriteInSummary } from './tabulation/write_ins';
import { rootDebug } from './util/debug';
import { tabulateTallyReportResults } from './tabulation/tally_reports';
import { buildExporter } from './util/exporter';
import { importCastVoteRecords } from './cast_vote_records';

const debug = rootDebug.extend('app');

Expand Down Expand Up @@ -404,9 +408,39 @@ function buildApi({
}): Promise<
Result<
CvrFileImportInfo,
AddCastVoteRecordReportError & { message: string }
| (AddCastVoteRecordReportError & { message: string })
| ImportCastVoteRecordsError
>
> {
/* c8 ignore start */
if (
isFeatureFlagEnabled(
BooleanEnvironmentVariableName.ENABLE_CONTINUOUS_EXPORT
)
) {
const userRole = assertDefined(await getUserRole());
const importResult = await importCastVoteRecords(store, input.path);
Copy link
Collaborator

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.

Copy link
Contributor Author

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

Copy link
Collaborator

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.

Copy link
Contributor Author

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

if (importResult.isErr()) {
await logger.log(LogEventId.CvrLoaded, userRole, {
disposition: 'failure',
errorDetails: JSON.stringify(importResult.err()),
errorType: importResult.err().type,
exportDirectoryPath: input.path,
result: 'Cast vote records not imported, error shown to user.',
});
} else {
await logger.log(LogEventId.CvrLoaded, userRole, {
disposition: 'success',
exportDirectoryPath: input.path,
numberOfBallotsImported: importResult.ok().newlyAdded,
numberOfDuplicateBallotsIgnored: importResult.ok().alreadyPresent,
result: 'Cast vote records imported.',
});
}
return importResult;
}
/* c8 ignore stop */

const userRole = assertDefined(await getUserRole());
const { path: inputPath } = input;
// the path passed to the backend may be for the report directory or the
Expand Down