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

arsalansufi · 2023-10-02T15:30:35Z

Easiest reviewed by commit

Overview

Issue link: #3921 + #3922

This PR picks up where #4011 leaves off. It deprecates VxCentralScan's existing backup functionality in favor of "extended" CVR exports that contain images not only for accepted ballots but also rejected* ballots. It also by default has VxCentralScan export only the images that are absolutely necessary for tabulation (images for accepted ballots with write-ins).

Copying from that previous PR's description:

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 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).

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

arsalansufi · 2023-10-02T18:48:46Z

apps/central-scan/backend/schema.sql

@@ -8,7 +8,6 @@ create table election (
  polls_state text not null default "polls_closed_initial",
  ballot_count_when_ballot_bag_last_replaced integer not null default 0,
  is_sound_muted boolean not null default false,
-  cvrs_backed_up_at datetime,


This value and the corresponding setCvrsBackedUp and getCvrsBackupTimestamp values haven't been used in a while, as far as I can tell. scanner_backed_up_at, on the other hand, is still relevant

arsalansufi · 2023-10-02T18:52:15Z

apps/central-scan/frontend/src/components/export_results_modal.tsx

-        message: `Successfully saved CVR file with ${numberOfBallots} ballots.`,
-        disposition: 'success',
-        numberOfBallots,
-      });


I'm revisiting logging holistically in a follow-up PR (leaning toward consolidating logging in app backends)

arsalansufi · 2023-10-02T18:52:23Z

apps/central-scan/frontend/src/screens/admin_actions_screen.tsx

-        disposition: 'failure',
-        message: `Error saving ballot data backup: ${error.message}`,
-        result: 'No backup saved.',
-      });


Same comment here re logging

arsalansufi · 2023-10-02T21:25:06Z

apps/central-scan/backend/src/store.ts

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


The VxCentralScan UX guarantees this - sheets requiring review have to be accepted or rejected before a batch is considered complete. And if someone shuts the machine down mid-adjudication, on boot, incomplete batches are cleaned up:

vxsuite/apps/central-scan/backend/src/server.ts

Line 59 in 8b41fda

resolvedWorkspace.store.cleanupIncompleteBatches();

We've discussed changing how central scan does adjudication - maybe add a comment with a bit of this explanation so that anyone changing this in the future sees the context?

Good call! 0db5543

adghayes · 2023-10-03T11:23:31Z

apps/central-scan/backend/src/store.ts

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


We've discussed changing how central scan does adjudication - maybe add a comment with a bit of this explanation so that anyone changing this in the future sees the context?

adghayes · 2023-10-03T11:26:07Z

apps/central-scan/backend/src/central_scanner_app.ts

-  deprecatedApiRouter.post<
-    NoParams,
-    Scan.ExportToUsbDriveResponse,
-    Scan.ExportToUsbDriveRequest
-  >('/central-scanner/scan/export-to-usb-drive', async (_request, response) => {


Getting slightly closer to 100% grout, nice!

Where "minimal" means that the export includes only the images that are absolutely necessary for tabulation (images for accepted ballots with write-ins)

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

arsalansufi force-pushed the arsalan/central-scan-backup branch 3 times, most recently from dfdb2e9 to 26a9efe Compare October 2, 2023 18:46

arsalansufi commented Oct 2, 2023

View reviewed changes

arsalansufi requested a review from adghayes October 2, 2023 19:22

arsalansufi marked this pull request as ready for review October 2, 2023 19:22

arsalansufi requested a review from a team as a code owner October 2, 2023 19:22

arsalansufi requested review from eventualbuddha and removed request for a team and eventualbuddha October 2, 2023 19:22

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

arsalansufi force-pushed the arsalan/central-scan-backup branch from b416d51 to d68f8d5 Compare October 2, 2023 21:22

arsalansufi commented Oct 2, 2023

View reviewed changes

adghayes approved these changes Oct 3, 2023

View reviewed changes

Base automatically changed from arsalan/rejected-images to main October 4, 2023 15:22

arsalansufi added 11 commits October 4, 2023 11:23

Add support for "minimal" CVR exports

032684c

Where "minimal" means that the export includes only the images that are absolutely necessary for tabulation (images for accepted ballots with write-ins)

Update VxCentralScan backend store, specifically:

8c2742c

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

Update VxCentralScan backend store tests

b000ff9

Consolidate CVR export and backup logic in VxCentralScan backend app

e354f92

Update remaining VxCentralScan backend tests

23e06d0

Update VxCentralScan backend code coverage thresholds

d43d2c9

Update VxCentralScan frontend

e0addb9

Update VxCentralScan frontend tests

4cc1d73

Update VxCentralScan frontend code coverage thresholds

d676c1a

Add clarifying comments

3bac94a

Add comments explaining why condition is guaranteed

0db5543

arsalansufi force-pushed the arsalan/central-scan-backup branch from d68f8d5 to 0db5543 Compare October 4, 2023 15:24

Fix logical merge conflict

6f5a45f

arsalansufi merged commit cdf1445 into main Oct 4, 2023

arsalansufi deleted the arsalan/central-scan-backup branch October 4, 2023 15:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

arsalansufi commented Oct 2, 2023 •

edited

Loading

arsalansufi Oct 2, 2023

arsalansufi Oct 2, 2023

arsalansufi Oct 2, 2023

arsalansufi Oct 2, 2023

adghayes Oct 3, 2023

arsalansufi Oct 4, 2023

adghayes Oct 3, 2023

adghayes Oct 3, 2023

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

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

Conversation

arsalansufi commented Oct 2, 2023 • edited Loading

Overview

Caveat

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

arsalansufi commented Oct 2, 2023 •

edited

Loading