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

Deprecate VxScan's existing backup functionality in favor of "extended" CVR exports #4011

Merged
merged 17 commits into from
Oct 4, 2023
Merged

Deprecate VxScan's existing backup functionality in favor of "extended" CVR exports #4011

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
f01ae9a
Prep for addition of rejected-<id>/ sub-directories in CVR exports
arsalansufi Sep 30, 2023
0e8d42c
Clean up image reference logic in buildCastVoteRecord fn, specifically:
arsalansufi Sep 30, 2023
da36836
Update buildCastVoteRecord tests
arsalansufi Sep 30, 2023
4bc4f60
Update exportCastVoteRecordsToUsbDrive to include rejected sheets
arsalansufi Sep 30, 2023
fb2ea1c
Update VxScan backend store, specifically:
arsalansufi Sep 30, 2023
c30f068
Update VxScan backend store tests
arsalansufi Sep 30, 2023
a74a6ef
Update VxScan state machine to export rejected sheets
arsalansufi Sep 30, 2023
455d5fc
Update rest of VxScan backend to account for store changes
arsalansufi Sep 30, 2023
f4537fa
Update remaining VxScan backend tests
arsalansufi Sep 30, 2023
7dcf49a
Update VxScan backend code coverage thresholds
arsalansufi Sep 30, 2023
64f051f
Remove now unneeded "Save Backup" button from VxScan UI
arsalansufi Sep 30, 2023
fb4deb7
Update VxScan frontend tests
arsalansufi Sep 30, 2023
3856417
Minimally update the VxCentralScan backend
arsalansufi Sep 30, 2023
8f7d6e1
Don't check whether USB requires CVR sync after polls have been closed
arsalansufi Oct 4, 2023
9615ad3
Block relevant actions when a CVR sync is detected as necessary
arsalansufi Oct 4, 2023
4e2090f
Update/add tests
arsalansufi Oct 4, 2023
6731e39
Add a few more tests
arsalansufi Oct 4, 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
2 changes: 0 additions & 2 deletions apps/scan/backend/schema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ create table election (
ballot_count_when_ballot_bag_last_replaced integer not null default 0,
is_sound_muted boolean not null default false,
is_ultrasonic_disabled boolean not null default false,
cvrs_backed_up_at datetime,
scanner_backed_up_at datetime,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not only can we delete logic for the old-style of backup; we can also delete logic for the check that prevents unconfiguring a machine until you've backed. Continuous export makes it such that you're essentially always backed up

created_at timestamp not null default current_timestamp
);

Expand Down
246 changes: 68 additions & 178 deletions apps/scan/backend/src/store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,16 @@ import {
safeParseSystemSettings,
AdjudicationReason,
} from '@votingworks/types';
import { assertDefined, Optional } from '@votingworks/basics';
import { assert, assertDefined, Optional } from '@votingworks/basics';
import * as fs from 'fs-extra';
import { sha256 } from 'js-sha256';
import { DateTime } from 'luxon';
import { join } from 'path';
import { v4 as uuid } from 'uuid';
import {
ResultSheet,
AcceptedSheet,
RejectedSheet,
Sheet,
UiStringsStore,
createUiStringStore,
} from '@votingworks/backend';
Expand All @@ -49,41 +51,39 @@ const debug = rootDebug.extend('store');

const SchemaPath = join(__dirname, '../schema.sql');

function dateTimeFromNoOffsetSqliteDate(noOffsetSqliteDate: string): DateTime {
return DateTime.fromFormat(noOffsetSqliteDate, 'yyyy-MM-dd HH:mm:ss', {
zone: 'GMT',
});
}

const getResultSheetsBaseQuery = `
const getSheetsBaseQuery = `
select
sheets.id as id,
batches.id as batchId,
batches.label as batchLabel,
front_interpretation_json as frontInterpretationJson,
back_interpretation_json as backInterpretationJson,
front_image_path as frontImagePath,
back_image_path as backImagePath
back_image_path as backImagePath,
requires_adjudication as requiresAdjudication,
finished_adjudication_at as finishedAdjudicationAt,
sheets.deleted_at as deletedAt
from sheets left join batches on
sheets.batch_id = batches.id
where
(requires_adjudication = 0 or finished_adjudication_at is not null)
and sheets.deleted_at is null
and batches.deleted_at is null
`;

interface ResultSheetRow {
interface SheetRow {
id: string;
batchId: string;
batchLabel: string | null;
frontInterpretationJson: string;
backInterpretationJson: string;
frontImagePath: string;
backImagePath: string;
requiresAdjudication: 0 | 1;
finishedAdjudicationAt: Iso8601Timestamp | null;
deletedAt: Iso8601Timestamp | null;
}

function resultSheetRowToResultSheet(row: ResultSheetRow): ResultSheet {
function sheetRowToAcceptedSheet(row: SheetRow): AcceptedSheet {
assert(row.deletedAt === null);
return {
type: 'accepted',
id: row.id,
batchId: row.batchId,
batchLabel: row.batchLabel ?? undefined,
Expand All @@ -96,6 +96,28 @@ function resultSheetRowToResultSheet(row: ResultSheetRow): ResultSheet {
};
}

function sheetRowToRejectedSheet(row: SheetRow): RejectedSheet {
assert(row.deletedAt !== null);
return {
type: 'rejected',
id: row.id,
frontImagePath: row.frontImagePath,
backImagePath: row.backImagePath,
};
}

function sheetRowToSheet(row: SheetRow): Sheet {
assert(
row.requiresAdjudication === 0 ||
row.finishedAdjudicationAt !== null ||
row.deletedAt !== null,
'Every sheet requiring review should have been either accepted or rejected'
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition is guaranteed to hold given transactions in a subsequent commit

return row.deletedAt === null
? sheetRowToAcceptedSheet(row)
: sheetRowToRejectedSheet(row);
}

/**
* Manages a data store for imported ballot image batches and cast vote records
* interpreted by reading the sheets.
Expand Down Expand Up @@ -466,68 +488,6 @@ export class Store {
return ongoingBatchRow?.id;
}

/**
* Records that batches have been backed up.
*/
setScannerBackedUp(backedUp = true): void {
if (!this.hasElection()) {
throw new Error('Unconfigured scanner cannot be backed up.');
}

if (backedUp) {
this.client.run(
'update election set scanner_backed_up_at = current_timestamp'
);
} else {
this.client.run('update election set scanner_backed_up_at = null');
}
}

/**
* Records that CVRs have been backed up.
*/
setCvrsBackedUp(backedUp = true): void {
if (!this.hasElection()) {
throw new Error('Unconfigured scanner cannot export CVRs.');
}

if (backedUp) {
this.client.run(
'update election set cvrs_backed_up_at = current_timestamp'
);
} else {
this.client.run('update election set cvrs_backed_up_at = null');
}
}

/**
* Gets the timestamp for the last scanner backup
*/
getScannerBackupTimestamp(): DateTime | undefined {
const row = this.client.one(
'select scanner_backed_up_at as scannerBackedUpAt from election'
) as { scannerBackedUpAt: string } | undefined;
if (!row?.scannerBackedUpAt) {
return undefined;
}

return dateTimeFromNoOffsetSqliteDate(row?.scannerBackedUpAt);
}

/**
* Gets the timestamp for the last cvr export
*/
getCvrsBackupTimestamp(): DateTime | undefined {
const row = this.client.one(
'select cvrs_backed_up_at as cvrsBackedUpAt from election'
) as { cvrsBackedUpAt: string } | undefined;
if (!row?.cvrsBackedUpAt) {
return undefined;
}

return dateTimeFromNoOffsetSqliteDate(row?.cvrsBackedUpAt);
}

getBallotsCounted(): number {
const row = this.client.one(`
select
Expand All @@ -545,64 +505,6 @@ export class Store {
return row?.ballotsCounted ?? 0;
}

/**
* Returns whether the appropriate backups have been completed and it is safe
* to unconfigure a machine / reset the election session. Always returns
* true in test mode.
*/
getCanUnconfigure(): boolean {
Comment on lines -548 to -553
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so now that we have continuous backup you can always reconfigure because it's always backed up. Yes?

When the USB is out of sync and needs to be updated it shows a warning and you click a button to sync. But IIRC it shows the prompt via the voter screen, and the election manager screens are still accessible normally with authentication. So does this mean that you could have the "out-of-sync" warning, insert an Election Manager card, and unconfigure anyway? I haven't confirmed this is actually possible so may just be missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line of reasoning is all correct - it is possible to access the PW, EM, and SA screens when the sync prompt is up on the voter screen.

I'd been thinking about how worth it it was to prevent unconfiguring while in this fairly rare state. Things are complicated by the fact that the USB likely won't be in the machine come time to unconfigure, since it'll have been taken to VxAdmin for tabulation. And we don't display the sync prompt when there's no USB in the machine at all. Which brings us back to the need for some separate piece of store state if we want to fully track this.

I'm leaning toward disabling the PW polls-close button and the EM delete-election-data button when in the sync-required state, as the simplest possible safeguard.

Yes, an EM could still remove the USB and then delete election data (since the EM screen is still accessible when there's no USB). But a PW couldn't remove the USB and then close polls (since the PW screen is not accessible when there's no USB). So if a machine has had its polls closed, it's guaranteed to be backed up.

I think that this covers our bases well but can double check with Roe, as well!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to have @mattroe think this through too, but your explanation makes sense to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Posted to Slack

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And got confirmation that we're good to go with the edits that I've proposed in this thread ✅

Copy link
Contributor Author

@arsalansufi arsalansufi Oct 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

9615ad3 + 4e2090f + 6731e39

This thread made me realize that there are quite a few operations that we need to block when a CVR sync is detected as necessary:

  • Switching from official mode to test mode (assuming ballots have been counted)
  • Deleting election data
  • Closing polls (including from the polls paused state)

This video demonstrates all of the above:

checks-everywhere.mov

While I was at it, I also had to tweak the flow for switching from official mode to test mode when ballots have been counted, which used to direct users to the "Save Backup" button. I've swapped that modal out with a simple confirmation modal:

switch-mode.mov

// Always allow in test mode
if (this.getTestMode()) {
return true;
}

// Allow if no ballots have been counted
if (!this.getBallotsCounted()) {
return true;
}

const scannerBackedUpAt = this.getScannerBackupTimestamp();

// Require that a scanner backup has taken place
if (!scannerBackedUpAt) {
return false;
}

// Adding or deleting sheets would have updated the CVR count
const { maxSheetsCreatedAt, maxSheetsDeletedAt } = this.client.one(`
select
max(created_at) as maxSheetsCreatedAt,
max(deleted_at) as maxSheetsDeletedAt
from sheets
`) as {
maxSheetsCreatedAt: string;
maxSheetsDeletedAt: string;
};

// Deleting non-empty batches would have updated the CVR count
const { maxBatchesDeletedAt } = this.client.one(`
select
max(batches.deleted_at) as maxBatchesDeletedAt
from batches inner join sheets
on sheets.batch_id = batches.id
where sheets.deleted_at is null
`) as {
maxBatchesDeletedAt: string;
};

const cvrsLastUpdatedDates = [
maxBatchesDeletedAt,
maxSheetsCreatedAt,
maxSheetsDeletedAt,
]
.filter(Boolean)
.map((noOffsetSqliteDate) =>
dateTimeFromNoOffsetSqliteDate(noOffsetSqliteDate)
);

return scannerBackedUpAt >= DateTime.max(...cvrsLastUpdatedDates);
}

/**
* Adds a sheet to an existing batch.
*/
Expand Down Expand Up @@ -681,8 +583,6 @@ export class Store {
this.client.transaction(() => {
this.setPollsState('polls_closed_initial');
this.setBallotCountWhenBallotBagLastReplaced(0);
this.setCvrsBackedUp(false);
this.setScannerBackedUp(false);
});
}
// delete batches, which will cascade delete sheets
Expand Down Expand Up @@ -741,35 +641,6 @@ export class Store {
debug('no review sheets requiring adjudication');
}

*getSheets(): Generator<{
id: string;
frontImagePath: string;
backImagePath: string;
exportedAsCvrAt: Iso8601Timestamp;
}> {
for (const { id, frontImagePath, backImagePath, exportedAsCvrAt } of this
.client.each(`
select
id,
front_image_path as frontImagePath,
back_image_path as backImagePath
from sheets
order by created_at asc
`) as Iterable<{
id: string;
frontImagePath: string;
backImagePath: string;
exportedAsCvrAt: Iso8601Timestamp;
}>) {
yield {
id,
frontImagePath,
backImagePath,
exportedAsCvrAt,
};
}
}

adjudicateSheet(sheetId: string): boolean {
debug('finishing adjudication for sheet %s', sheetId);

Expand Down Expand Up @@ -888,23 +759,42 @@ export class Store {
}

/**
* Yields all sheets in the database that would be included in a CVR export.
* Yields all scanned sheets that were accepted and should be tabulated
*/
*forEachAcceptedSheet(): Generator<AcceptedSheet> {
const sql = `${getSheetsBaseQuery}
where
batches.deleted_at is null and
sheets.deleted_at is null and
(requires_adjudication = 0 or finished_adjudication_at is not null)
order by sheets.id
`;
for (const row of this.client.each(sql) as Iterable<SheetRow>) {
yield sheetRowToAcceptedSheet(row);
}
}

/**
* Yields all scanned sheets
*/
*forEachResultSheet(): Generator<ResultSheet> {
const sql = `${getResultSheetsBaseQuery} order by sheets.id`;
for (const row of this.client.each(sql) as Iterable<ResultSheetRow>) {
yield resultSheetRowToResultSheet(row);
*forEachSheet(): Generator<Sheet> {
const sql = `${getSheetsBaseQuery}
where
batches.deleted_at is null
order by sheets.id
`;
for (const row of this.client.each(sql) as Iterable<SheetRow>) {
yield sheetRowToSheet(row);
}
}

/**
* Gets a single sheet given a sheet ID. Returns undefined if the sheet doesn't exist or wouldn't
* be included in a CVR export.
* Gets a sheet given a sheet ID. Returns undefined if the sheet doesn't exist.
*/
getResultSheet(sheetId: string): ResultSheet | undefined {
const sql = `${getResultSheetsBaseQuery} and sheets.id = ?`;
const row = this.client.one(sql, sheetId) as ResultSheetRow | undefined;
return row ? resultSheetRowToResultSheet(row) : undefined;
getSheet(sheetId: string): Sheet | undefined {
const sql = `${getSheetsBaseQuery} where sheets.id = ?`;
const row = this.client.one(sql, sheetId) as SheetRow | undefined;
return row ? sheetRowToSheet(row) : undefined;
}

/**
Expand Down