Save voter session on ballot spoil #4465

kshen0 · 2024-01-03T23:49:00Z

Overview

#3909

Improves the flow for spoiling a ballot in post-print ballot validation

Rather than restarting the voter session, the session is saved. The voter is returned to the review screen after the poll worker removes the old ballot, loads a new sheet, and removes their card
The ballot spoil flow now checks that the ballot is removed by the poll worker before allowing the poll worker to continue

Demo Video or Screenshot

Happy path

IMG_4641.mov

Lightly stress testing pulling paper early and repeated auth changes

IMG_4642.mov

Testing Plan

updated component and e2e tests

Checklist

I have added logging where appropriate to any new user actions, system updates such as file reads or storage writes, or errors introduced. (no new actions or errors introduced)

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

CLAassistant · 2024-01-03T23:49:06Z

All committers have signed the CLA.

render BallotInvalidatedPage outside ballot context because ballot context is no longer needed

Previously only a cardless voter session could render LoadPaperPage. We now also want to render it when poll worker auth is active, as will be the case when a poll worker has removed a ballot to spoil and needs to load a new sheet

Updates to tests in this PR require finer control of the mock state machine, so we don't want to always set the state machine state when setting expectations

kshen0 · 2024-01-05T05:41:37Z

apps/mark-scan/backend/src/custom-paper-handler/state_machine.ts

@@ -90,7 +90,7 @@ type PaperHandlerStatusEvent =
  | { type: 'VOTER_INITIATED_PRINT'; pdfData: Buffer }
  | { type: 'PAPER_IN_OUTPUT' }
  | { type: 'PAPER_IN_INPUT' }
-  | { type: 'VOTER_CONFIRMED_INVALIDATED_BALLOT' }
+  | { type: 'POLLWORKER_CONFIRMED_INVALIDATED_BALLOT' }


This was a misnomer; the voter is not actually the one who confirms the ballot is has been invalidated

kshen0 · 2024-01-05T05:42:23Z

apps/mark-scan/backend/src/custom-paper-handler/state_machine.ts

+                on: {
+                  POLLWORKER_CONFIRMED_INVALIDATED_BALLOT: 'done',
+                },
+              },


The copy changes based on whether paper is present

paper_nowhere is a little confusing. Maybe no_paper or paper_absent?

kshen0 · 2024-01-05T05:43:31Z

apps/mark-scan/frontend/src/app_cardless_voting.test.tsx

@@ -33,7 +33,7 @@ beforeEach(() => {
  apiMock = createApiMock();
  kiosk = fakeKiosk();
  window.kiosk = kiosk;
-  apiMock.setPaperHandlerState('waiting_for_ballot_data');


These e2e tests "incorrectly" mocked paper handler state; the old mocking seen here was simpler but a less accurate representation of states at various points of the e2e test. It wasn't an issue before now because of the order we rendered LoadPaperPage in (ie. it used to be lower priority)

kshen0 · 2024-01-05T05:45:35Z

apps/mark-scan/frontend/src/app_root.tsx

+          paperPresent={
+            stateMachineState ===
+            'waiting_for_invalidated_ballot_confirmation.paper_present'
+          }


Seemed clearer to pass this prop down rather than query for state in the lower components, but open to feedback

seems fine to me!

kshen0 · 2024-01-05T05:46:08Z

apps/mark-scan/frontend/src/app_root.tsx

-          ballotContextProviderChild = (
-            <BallotInvalidatedPage authStatus={authStatus} />
-          );
-        }


Since we are no longer restarting the voting session, we don't need access to BallotContext

kshen0 · 2024-01-05T05:46:41Z

apps/mark-scan/frontend/src/pages/poll_worker_screen.tsx

-                              onSuccess() {
-                                setSelectedCardlessVoterPrecinctId(precinct.id);
-                              },
-                            });


This was redundant with another call to setAcceptingPaperStateMutation.mutate that happens after ballot style selection

kshen0 · 2024-01-05T17:37:31Z

apps/mark-scan/frontend/src/pages/remove_invalidated_ballot_page.tsx

+  const history = useHistory();
+  useEffect(() => {
+    history.push('/ready-to-review');
+  });


As discussed before, not ideal to use react-router routing here. Should be simplified in #3985

jonahkagan

Thanks for the clear PR!

jonahkagan · 2024-01-08T21:44:38Z

apps/mark-scan/backend/src/custom-paper-handler/state_machine.ts

+                on: {
+                  POLLWORKER_CONFIRMED_INVALIDATED_BALLOT: 'done',
+                },
+              },


paper_nowhere is a little confusing. Maybe no_paper or paper_absent?

jonahkagan · 2024-01-08T21:46:01Z

apps/mark-scan/frontend/src/app_root.tsx

+          paperPresent={
+            stateMachineState ===
+            'waiting_for_invalidated_ballot_confirmation.paper_present'
+          }


seems fine to me!

jonahkagan · 2024-01-08T21:53:04Z

apps/mark-scan/frontend/src/pages/ballot_invalidated_page.tsx

  return (
    <AskPollWorkerPage
      authStatus={authStatus}
-      pollWorkerPage={<RemoveInvalidatedBallotPage />}
+      pollWorkerPage={
+        <RemoveInvalidatedBallotPage paperPresent={paperPresent} />


I noticed in the video that the "Ask a Poll Worker for Help" text wraps awkwardly to two lines. I think we should look into moving the left-aligned text. Also, the "Voting Session in Progress" screen could use some more horizontal padding - it's butting right up against the sides. But these are minor issues that can be addressed in a future UI polish pass.

The "Ask a Poll Worker for Help" screen uses the CenteredPageLayout component, which is used by a lot of the app. Agree we should do a design pass on that component and make sure we update all callers at once for consistency.

"Voting Session in Progress" padding was a one-off fix though:

Before

After

It has the same wrapping awkwardness with padding, but at least it's consistent so IMO it's the lesser of two evils. We can fix this and other screens in https://github.com/orgs/votingworks/projects/23/views/75?pane=issue&itemId=49471078

kshen0 added 11 commits January 4, 2024 21:06

clean up debug statements

cb0c7b3

split ballot invalidation state into 2 substates

edb5925

add new states to types

2a575d4

use new states and BallotInvalidatedPage interface in app_root

05ffaa0

render BallotInvalidatedPage outside ballot context because ballot context is no longer needed

pass paperPresent prop down

c94fe0d

change ballot invalidation instructions based on paper status

9b97245

render LoadPaperPage regardless of auth status

a79ef43

Previously only a cardless voter session could render LoadPaperPage. We now also want to render it when poll worker auth is active, as will be the case when a poll worker has removed a ballot to spoil and needs to load a new sheet

decouple expectation and mock state machine update

26b27e5

Updates to tests in this PR require finer control of the mock state machine, so we don't want to always set the state machine state when setting expectations

update BallotInvalidatedPage tests

6fb4073

update app e2e tests

9d4ab4d

update ballot invalidation copy

9adeae4

kshen0 force-pushed the kevin/save-voter-session-on-ballot-spoil-3909 branch from bf7d9e0 to 9adeae4 Compare January 5, 2024 05:26

kshen0 added the user_facing_change label Jan 5, 2024

kshen0 commented Jan 5, 2024

View reviewed changes

kshen0 changed the title ~~WIP Kevin/save voter session on ballot spoil 3909~~ Save voter session on ballot spoil Jan 5, 2024

kshen0 requested a review from jonahkagan January 5, 2024 05:49

kshen0 commented Jan 5, 2024

View reviewed changes

kshen0 marked this pull request as ready for review January 5, 2024 19:39

jonahkagan approved these changes Jan 8, 2024

View reviewed changes

kshen0 added 2 commits January 9, 2024 11:15

s/paper_nowhere/paper_absent

f16c6ec

add padding to "Voting Session in Progress" screen

682b207

kshen0 merged commit d4b1251 into main Jan 9, 2024
51 checks passed

kshen0 deleted the kevin/save-voter-session-on-ballot-spoil-3909 branch January 9, 2024 21:22

kshen0 mentioned this pull request Jan 9, 2024

Return to review screen after spoiling ballot #3909

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Save voter session on ballot spoil #4465

Save voter session on ballot spoil #4465

kshen0 commented Jan 3, 2024 •

edited

Loading

CLAassistant commented Jan 3, 2024 •

edited

Loading

kshen0 Jan 5, 2024

kshen0 Jan 5, 2024

jonahkagan Jan 8, 2024

kshen0 Jan 5, 2024 •

edited

Loading

kshen0 Jan 5, 2024

jonahkagan Jan 8, 2024

kshen0 Jan 5, 2024

kshen0 Jan 5, 2024

kshen0 Jan 5, 2024

jonahkagan left a comment

jonahkagan Jan 8, 2024

jonahkagan Jan 8, 2024

jonahkagan Jan 8, 2024

kshen0 Jan 9, 2024

Save voter session on ballot spoil #4465

Save voter session on ballot spoil #4465

Conversation

kshen0 commented Jan 3, 2024 • edited Loading

Overview

Demo Video or Screenshot

Testing Plan

Checklist

CLAassistant commented Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kshen0 Jan 5, 2024 • edited Loading

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

jonahkagan left a 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

kshen0 commented Jan 3, 2024 •

edited

Loading

CLAassistant commented Jan 3, 2024 •

edited

Loading

kshen0 Jan 5, 2024 •

edited

Loading