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

arsalansufi · 2023-10-02T14:34:16Z

Easiest reviewed by commit

Overview

Issue link: #3922

This PR deprecates VxScan's existing backup functionality in favor of "extended" CVR exports that contain images not only for accepted ballots but also rejected* ballots. (This is currently the key delta between CVR exports and backups - backups include rejected ballot images. There are other deltas, like backups' inclusion of the DB file, but we want to drop that for privacy reasons.)

In my next PR, I'll focus on VxCentralScan, where I'll also deprecate existing backup functionality in favor of extended CVR exports, but will also by default have VxCentralScan export only the images that are absolutely necessary for tabulation (images for accepted ballots with write-ins).

Summarizing, when all is said and done:

VxScan continuous export will include images for all ballots, accepted and rejected
VxScan "Save CVRs" button will include images for all ballots, accepted and rejected (same as 1)
VxScan will no longer have a "Save Backup" button
VxCentralScan "Save CVRs" button will include images for accepted ballots with write-ins
VxCentralScan "Save Backup" button will include images for all ballots, accepted and rejected (same as 1 and 2)

This PR handles 1, 2, and 3. My next PR will handle 4 and 5.

Caveat

*By "rejected" ballots, I mean interpretable rejected ballots. Tracking and exporting all rejected ballots, including ones that we couldn't interpret at all, will require some extra work. Today's backups also don't include these images, so we're not losing anything with this PR (Slack thread with some context).

Demo Screenshot and Video

No more "Save Backup" button, just "Save CVRs"

CVR export contents, including "rejected-" sub-directories

contents.mov

Testing

Updated automated tests
Tested manually

Checklist

~~I have added logging where appropriate to any new user actions, system updates such as file reads or storage writes, or errors introduced~~ Revisiting logging holistically in a follow-up PR
I have added a screenshot and/or video to this PR to demo the change
I have added the "user_facing_change" label to this PR to automate an announcement in #machine-product-updates

arsalansufi · 2023-10-02T17:31:14Z

libs/backend/src/cast_vote_records/build_cast_vote_record.ts

+      ? imageFileUris.map((imageFileUri) => ({
+          '@type': 'CVR.ImageData',
+          Location: imageFileUri,
+        }))


Following up on our discussion in #3988 (comment) re including either no images or both front and back images, but never just one image

arsalansufi · 2023-10-02T17:33:21Z

libs/backend/src/cast_vote_records/export.ts

 */
 async function randomlyUpdateCreationTimestamps(
  exportContext: ExportContext,
  exportDirectoryPathRelativeToUsbMountPoint: string,
-  options: { castVoteRecordIdToIgnore?: string } = {}
+  options: { subDirectoryNameToIgnore?: string } = {}


Updated the variable names in this function because we're now shuffling not only cast vote record sub-directories but also sub-directories for rejected sheets, i.e. all export sub-directories

arsalansufi · 2023-10-02T17:35:26Z

apps/scan/backend/schema.sql

@@ -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,


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

arsalansufi · 2023-10-02T17:36:28Z

apps/scan/backend/src/store.ts

+      row.finishedAdjudicationAt !== null ||
+      row.deletedAt !== null,
+    'Every sheet requiring review should have been either accepted or rejected'
+  );


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

arsalansufi · 2023-10-02T17:40:48Z

apps/scan/backend/test/helpers/shared_helpers.ts

- *   ballot interpretation (because the state machine doesn't actually use those
- *   page interpretations, they are just stored for the CVR).
- */
-export function mockInterpret(


We can't use this function any more because the second condition in the above list re CVR exports and ballot images is no longer met. We're now always exporting ballot images after scans (aside from the caveat mentioned in the PR description).

For rejected sheets

- Include image references for BMD ballots - Include image references for both sides of a sheet (instead of potentially just one side) to simplify the data model

Also account for changes to buildCastVoteRecord

- Update sheet retrieval methods - Delete old-style backup methods

To account for libs/backend changes

adghayes · 2023-10-03T08:31:30Z

libs/backend/src/cast_vote_records/export.ts

@@ -133,6 +133,21 @@ export interface ResultSheet {
  readonly batchLabel?: string;


sidebar: is this still needed?

I'm not entirely sure - left a note to revisit this in my temporary catch-all issue: #3996

adghayes · 2023-10-03T08:46:35Z

apps/scan/backend/src/store.ts

-  /**
-   * 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 {


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.

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!

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

👍 Posted to Slack

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

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

adghayes · 2023-10-03T15:32:12Z

@arsalansufi one more thought about replacing backups... backups do / did include the log file, now our replacement doesn't include the log file. So that may be lost, and people will probably forget to export it. Not sure if that matters, but may be worth a Slack thread if there hasn't been one already.

arsalansufi · 2023-10-03T15:44:11Z

one more thought about replacing backups... backups do / did include the log file, now our replacement doesn't include the log file. So that may be lost, and people will probably forget to export it. Not sure if that matters, but may be worth a Slack thread if there hasn't been one already.

Yeah another good point. This has come up in past discussion, and I believe that we decided that it was okay for log export to remain separate. But I re-raised this in Slack, too

As well as before polls have been opened

- Closing polls - Switching from official mode to test mode (assuming ballots have been counted) - Deleting election data

arsalansufi added the user_facing_change label Oct 2, 2023

arsalansufi mentioned this pull request Oct 2, 2023

Deprecate VxCentralScan's existing backup functionality in favor of "extended" CVR exports #4012

Merged

3 tasks

arsalansufi commented Oct 2, 2023

View reviewed changes

arsalansufi requested a review from adghayes October 2, 2023 17:41

arsalansufi marked this pull request as ready for review October 2, 2023 17:41

arsalansufi requested a review from a team as a code owner October 2, 2023 17:41

arsalansufi requested review from kshen0 and removed request for a team and kshen0 October 2, 2023 17:41

Base automatically changed from arsalan/more-robust-waiting to main October 2, 2023 21:17

arsalansufi added 13 commits October 2, 2023 17:19

Prep for addition of rejected-<id>/ sub-directories in CVR exports

f01ae9a

For rejected sheets

Clean up image reference logic in buildCastVoteRecord fn, specifically:

0e8d42c

- Include image references for BMD ballots - Include image references for both sides of a sheet (instead of potentially just one side) to simplify the data model

Update buildCastVoteRecord tests

da36836

Update exportCastVoteRecordsToUsbDrive to include rejected sheets

4bc4f60

Also account for changes to buildCastVoteRecord

Update VxScan backend store, specifically:

fb2ea1c

- Update sheet retrieval methods - Delete old-style backup methods

Update VxScan backend store tests

c30f068

Update VxScan state machine to export rejected sheets

a74a6ef

Update rest of VxScan backend to account for store changes

455d5fc

Update remaining VxScan backend tests

f4537fa

Update VxScan backend code coverage thresholds

7dcf49a

Remove now unneeded "Save Backup" button from VxScan UI

64f051f

Update VxScan frontend tests

fb4deb7

Minimally update the VxCentralScan backend

3856417

To account for libs/backend changes

arsalansufi force-pushed the arsalan/rejected-images branch from 81e4323 to 3856417 Compare October 2, 2023 21:19

adghayes approved these changes Oct 3, 2023

View reviewed changes

arsalansufi added 3 commits October 4, 2023 10:36

Don't check whether USB requires CVR sync after polls have been closed

8f7d6e1

As well as before polls have been opened

Block relevant actions when a CVR sync is detected as necessary

9615ad3

- Closing polls - Switching from official mode to test mode (assuming ballots have been counted) - Deleting election data

Update/add tests

4e2090f

Add a few more tests

6731e39

arsalansufi merged commit d5e4f83 into main Oct 4, 2023
50 checks passed

arsalansufi deleted the arsalan/rejected-images branch October 4, 2023 15:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

arsalansufi commented Oct 2, 2023 •

edited

Loading

arsalansufi Oct 2, 2023

arsalansufi Oct 2, 2023

arsalansufi Oct 2, 2023

arsalansufi Oct 2, 2023

arsalansufi Oct 2, 2023

adghayes Oct 3, 2023

arsalansufi Oct 3, 2023

adghayes Oct 3, 2023

arsalansufi Oct 3, 2023

adghayes Oct 3, 2023

arsalansufi Oct 3, 2023

arsalansufi Oct 3, 2023

arsalansufi Oct 4, 2023 •

edited

Loading

adghayes commented Oct 3, 2023

arsalansufi commented Oct 3, 2023 •

edited

Loading

		@@ -133,6 +133,21 @@ export interface ResultSheet {
		readonly batchLabel?: string;

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

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

Conversation

arsalansufi commented Oct 2, 2023 • edited Loading

Overview

Caveat

Demo Screenshot and Video

Testing

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arsalansufi Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

adghayes commented Oct 3, 2023

arsalansufi commented Oct 3, 2023 • edited Loading

arsalansufi commented Oct 2, 2023 •

edited

Loading

arsalansufi Oct 4, 2023 •

edited

Loading

arsalansufi commented Oct 3, 2023 •

edited

Loading