Skip to content

Commit

Permalink
Polish CVR import and export UIs (#4047)
Browse files Browse the repository at this point in the history
* Restore ability to manually select a CVR file/directory for import

* Polish CVR import and export UIs, specifically:

- Remove references to singular CVR files, since CVR exports now
  consist of multiple files
- Make copy and UX more consistent and concise
- Don't allow closing saving/loading modals by clicking outside
  them
- Remove uses of deprecated Prose component and non-libs/ui Loading
  component

* Update tests

* Adjust VxCentralScan frontend code coverage thresholds

To account for removal of <Prose> and <Loading>

* Make sure saving backup on VxCentralScan doesn't crash app when no USB

Also clear error message on subsequent backup attempts

* Update integration tests

* Format large numbers with commas, in CVR import modal
  • Loading branch information
arsalansufi authored Oct 6, 2023
1 parent 0d8ff9f commit dc86724
Show file tree
Hide file tree
Showing 19 changed files with 156 additions and 253 deletions.
22 changes: 20 additions & 2 deletions apps/admin/backend/src/app.cvr_files.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@ import {
electionTwoPartyPrimaryFixtures,
} from '@votingworks/fixtures';
import { LogEventId } from '@votingworks/logging';
import { CVR, CVR as CVRType } from '@votingworks/types';
import { basename } from 'path';
import {
CVR,
CVR as CVRType,
CastVoteRecordExportFileName,
} from '@votingworks/types';
import path, { basename } from 'path';
import { vxBallotType } from '@votingworks/types/src/cdf/cast-vote-records';
import {
BooleanEnvironmentVariableName,
Expand Down Expand Up @@ -605,3 +609,17 @@ test('error if cast vote records from different files share same ballot id but h
expect(await apiClient.getCastVoteRecordFiles()).toHaveLength(1);
await expectCastVoteRecordCount(apiClient, 184);
});

test('specifying path to metadata file instead of path to export directory (for manual file selection)', async () => {
const { apiClient, auth } = buildTestEnvironment();
await configureMachine(apiClient, auth, electionDefinition);
mockElectionManagerAuth(auth, electionDefinition.electionHash);

const importResult = await apiClient.addCastVoteRecordFile({
path: path.join(
castVoteRecordExport.asDirectoryPath(),
CastVoteRecordExportFileName.METADATA
),
});
expect(importResult.isOk()).toEqual(true);
});
18 changes: 14 additions & 4 deletions apps/admin/backend/src/app.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { LogEventId, Logger } from '@votingworks/logging';
import {
BallotPackageFileName,
CastVoteRecordExportFileName,
ContestId,
DEFAULT_SYSTEM_SETTINGS,
Id,
Expand Down Expand Up @@ -30,7 +31,7 @@ import {
import * as grout from '@votingworks/grout';
import { useDevDockRouter } from '@votingworks/dev-dock-backend';
import { createReadStream, createWriteStream, promises as fs } from 'fs';
import { join } from 'path';
import path, { join } from 'path';
import {
BALLOT_PACKAGE_FOLDER,
generateFilenameForBallotExportPackage,
Expand Down Expand Up @@ -433,12 +434,21 @@ function buildApi({
await logger.log(LogEventId.ImportCastVoteRecordsInit, userRole, {
message: 'Importing cast vote records...',
});
const importResult = await importCastVoteRecords(store, input.path);
const exportDirectoryPath =
// For manual export selection, users must select the contained metadata file as a proxy
// for the export directory since the UI doesn't support directory selection
path.basename(input.path) === CastVoteRecordExportFileName.METADATA
? path.dirname(input.path)
: input.path;
const importResult = await importCastVoteRecords(
store,
exportDirectoryPath
);
if (importResult.isErr()) {
await logger.log(LogEventId.ImportCastVoteRecordsComplete, userRole, {
disposition: 'failure',
message: 'Error importing cast vote records.',
exportDirectoryPath: input.path,
exportDirectoryPath,
errorDetails: JSON.stringify(importResult.err()),
});
} else {
Expand All @@ -451,7 +461,7 @@ function buildApi({
await logger.log(LogEventId.ImportCastVoteRecordsComplete, userRole, {
disposition: 'success',
message,
exportDirectoryPath: input.path,
exportDirectoryPath,
});
}
return importResult;
Expand Down
12 changes: 6 additions & 6 deletions apps/admin/frontend/src/app.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -423,9 +423,9 @@ test('clearing results', async () => {

fireEvent.click(getByText('Tally'));
expect(
(await screen.findByText('Load CVR Files')).closest('button')
(await screen.findByText('Load CVRs')).closest('button')
).toBeDisabled();
expect(getByText('Remove CVR Files').closest('button')).toBeDisabled();
expect(getByText('Remove CVRs').closest('button')).toBeDisabled();
expect(
getByText('Edit Manually Entered Results').closest('button')
).toBeDisabled();
Expand All @@ -442,27 +442,27 @@ test('clearing results', async () => {
apiMock.expectGetManualResultsMetadata([]);
fireEvent.click(getByText('Clear All Tallies and Results'));
getByText(
'Do you want to remove the 1 loaded CVR file and the manually entered data?'
'Do you want to remove the 1 loaded CVR export and the manually entered data?'
);
fireEvent.click(getByText('Remove All Data'));

await waitFor(() => {
expect(getByText('Load CVR Files').closest('button')).toBeEnabled();
expect(getByText('Load CVRs').closest('button')).toBeEnabled();
});
await waitFor(() => {
expect(
getByText('Add Manually Entered Results').closest('button')
).toBeEnabled();
});

expect(getByText('Remove CVR Files').closest('button')).toBeDisabled();
expect(getByText('Remove CVRs').closest('button')).toBeDisabled();
expect(
getByText('Remove Manually Entered Results').closest('button')
).toBeDisabled();

expect(queryByText('Clear All Tallies and Results')).not.toBeInTheDocument();

getByText('No CVR files loaded.');
getByText('No CVRs loaded.');
});

test('can not view or print ballots', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ export function ConfirmRemovingFileModal({
let singleFileRemoval = true;
switch (fileType) {
case ResultsFileType.CastVoteRecord: {
fileTypeName = 'CVRs';
const fileList = castVoteRecordFilesQuery.data;
singleFileRemoval = fileList.length <= 1;
fileTypeName = 'CVR Files';
mainContent = (
<React.Fragment>
<P>
Do you want to remove the {fileList.length} loaded CVR{' '}
{pluralize('files', fileList.length)}?
{pluralize('export', fileList.length)}?
</P>
<P>All reports will be unavailable without CVR data.</P>
</React.Fragment>
Expand All @@ -56,7 +56,7 @@ export function ConfirmRemovingFileModal({
<React.Fragment>
<P>
Do you want to remove the {fileList.length} loaded CVR{' '}
{pluralize('files', fileList.length)}
{pluralize('export', fileList.length)}
{hasManualData && ' and the manually entered data'}?
</P>
<P>All reports will be unavailable without CVR data.</P>
Expand All @@ -70,7 +70,7 @@ export function ConfirmRemovingFileModal({

return (
<Modal
centerContent
title={`Remove ${fileTypeName}`}
content={mainContent}
actions={
<React.Fragment>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ describe('when USB is properly mounted', () => {
});
await waitFor(() =>
screen.getByText(
/There were no new CVR files automatically found on this USB drive/
/No new CVR exports were automatically found on this USB drive./
)
);

Expand All @@ -137,7 +137,7 @@ describe('when USB is properly mounted', () => {
apiMock.expectGetCastVoteRecordFileMode('test');
apiMock.expectGetCastVoteRecordFiles([]);

await screen.findByText('1,000 new CVRs Loaded');
await screen.findByText('1,000 New CVRs Loaded');

delete window.kiosk;
});
Expand All @@ -152,9 +152,9 @@ describe('when USB is properly mounted', () => {
usbDriveStatus: mockUsbDriveStatus('mounted'),
apiMock,
});
await screen.findByText('Load CVR Files');
await screen.findByText('Load CVRs');
screen.getByText(
/The following CVR files were automatically found on this USB drive./
/The following CVR exports were automatically found on this USB drive./
);

const tableRows = screen.getAllByTestId('table-row');
Expand Down Expand Up @@ -187,8 +187,8 @@ describe('when USB is properly mounted', () => {
apiMock.expectGetCastVoteRecordFiles([]);

userEvent.click(domGetByText(tableRows[0], 'Load'));
await screen.findByText('Loading');
await screen.findByText('1,000 new CVRs Loaded');
await screen.findByText('Loading CVRs');
await screen.findByText('1,000 New CVRs Loaded');
});

test('locks to test mode when in test mode & shows previously loaded files as loaded', async () => {
Expand All @@ -203,7 +203,7 @@ describe('when USB is properly mounted', () => {
apiMock,
});
await screen.findByRole('heading', {
name: 'Load Test Ballot Mode CVR Files',
name: 'Load Test Ballot Mode CVRs',
});

const tableRows = screen.getAllByTestId('table-row');
Expand Down Expand Up @@ -235,7 +235,7 @@ describe('when USB is properly mounted', () => {
usbDriveStatus: mockUsbDriveStatus('mounted'),
apiMock,
});
await screen.findByText('Load Official Ballot Mode CVR Files');
await screen.findByText('Load Official Ballot Mode CVRs');

const tableRows = screen.getAllByTestId('table-row');
expect(tableRows).toHaveLength(1);
Expand Down
83 changes: 48 additions & 35 deletions apps/admin/frontend/src/components/import_cvrfiles_modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -252,11 +252,11 @@ export function ImportCvrFilesModal({ onClose }: Props): JSX.Element | null {
if (currentState.state === 'duplicate') {
return (
<Modal
title="Duplicate File"
title="Duplicate Export"
content={
<P>
The selected file was ignored as a duplicate of a previously loaded
file.
The selected export was ignored as a duplicate of a previously
loaded export.
</P>
}
onOverlayClick={onClose}
Expand All @@ -266,25 +266,31 @@ export function ImportCvrFilesModal({ onClose }: Props): JSX.Element | null {
}

if (currentState.state === 'success') {
const { alreadyPresent, newlyAdded } = currentState.result;
const total = alreadyPresent + newlyAdded;
const content = (() => {
if (alreadyPresent > 0) {
if (total === 1) {
return <P>The 1 CVR in the selected export was previously loaded.</P>;
}
return (
<P>
Of the {format.count(total)} total CVRs in the selected export,{' '}
{format.count(alreadyPresent)}{' '}
{alreadyPresent === 1 ? 'was' : 'were'} previously loaded.
</P>
);
}
return <P>The CVRs in the selected export were successfully loaded.</P>;
})();
return (
<Modal
title={`${format.count(
currentState.result.newlyAdded
)} new CVRs Loaded`}
content={
currentState.result.alreadyPresent > 0 && (
<P>
Of the{' '}
{format.count(
currentState.result.newlyAdded +
currentState.result.alreadyPresent
)}{' '}
total CVRs in this file,{' '}
{format.count(currentState.result.alreadyPresent)} were previously
loaded.
</P>
)
title={
newlyAdded === 1
? '1 New CVR Loaded'
: `${format.count(newlyAdded)} New CVRs Loaded`
}
content={content}
onOverlayClick={onClose}
actions={<Button onPress={onClose}>Close</Button>}
/>
Expand All @@ -294,8 +300,7 @@ export function ImportCvrFilesModal({ onClose }: Props): JSX.Element | null {
if (
!castVoteRecordFilesQuery.isSuccess ||
!castVoteRecordFileModeQuery.isSuccess ||
!cvrFilesOnUsbQuery.isSuccess ||
currentState.state === 'loading'
!cvrFilesOnUsbQuery.isSuccess
) {
return (
<Modal
Expand All @@ -310,6 +315,10 @@ export function ImportCvrFilesModal({ onClose }: Props): JSX.Element | null {
);
}

if (currentState.state === 'loading') {
return <Modal content={<Loading>Loading CVRs</Loading>} />;
}

if (
usbDriveStatus.status === 'no_drive' ||
usbDriveStatus.status === 'ejected' ||
Expand All @@ -321,8 +330,7 @@ export function ImportCvrFilesModal({ onClose }: Props): JSX.Element | null {
content={
<P>
<UsbImage src="/assets/usb-drive.svg" alt="Insert USB Image" />
Please insert a USB drive in order to load CVR files from the
scanner.
Please insert a USB drive in order to load CVRs from a scanner.
</P>
}
onOverlayClick={onClose}
Expand All @@ -332,8 +340,9 @@ export function ImportCvrFilesModal({ onClose }: Props): JSX.Element | null {
<FileInputButton
data-testid="manual-input"
onChange={processCastVoteRecordFileFromFilePicker}
accept=".json"
>
Select Files
Select Export Manually
</FileInputButton>
)}
<Button onPress={onClose}>Cancel</Button>
Expand Down Expand Up @@ -367,7 +376,7 @@ export function ImportCvrFilesModal({ onClose }: Props): JSX.Element | null {
<tr key={name} data-testid="table-row">
<td>{moment(exportTimestamp).format(TIME_FORMAT)}</td>
<td>{scannerIds.join(', ')}</td>
<td data-testid="cvr-count">{cvrCount}</td>
<td data-testid="cvr-count">{format.count(cvrCount)}</td>
{!fileModeLocked && (
<td>
<LabelText>
Expand Down Expand Up @@ -410,29 +419,33 @@ export function ImportCvrFilesModal({ onClose }: Props): JSX.Element | null {
if (numberOfNewFiles === 0) {
instructionalText = fileModeLocked ? (
<React.Fragment>
There were no new {headerModeText} CVR files automatically found on
this USB drive. Save CVR files to this USB drive from the scanner.
Optionally, you may manually select files to load.
No new {headerModeText.toLowerCase()} CVR exports were automatically
found on this USB drive.
</React.Fragment>
) : (
'There were no new CVR files automatically found on this USB drive. Save CVR files to this USB drive from the scanner. Optionally, you may manually select files to load.'
<React.Fragment>
No new CVR exports were automatically found on this USB drive.
</React.Fragment>
);
} else if (fileModeLocked) {
instructionalText = (
<React.Fragment>
The following {headerModeText} CVR files were automatically found on
this USB drive. Previously loaded CVR entries will be ignored.
The following {headerModeText.toLowerCase()} CVR exports were
automatically found on this USB drive.
</React.Fragment>
);
} else {
instructionalText =
'The following CVR files were automatically found on this USB drive. Previously loaded CVR entries will be ignored.';
instructionalText = (
<React.Fragment>
The following CVR exports were automatically found on this USB drive.
</React.Fragment>
);
}

return (
<Modal
modalWidth={ModalWidth.Wide}
title={`Load ${headerModeText} CVR Files`}
title={`Load ${headerModeText} CVRs`}
content={
<Content>
<P>{instructionalText}</P>
Expand Down Expand Up @@ -462,7 +475,7 @@ export function ImportCvrFilesModal({ onClose }: Props): JSX.Element | null {
onChange={processCastVoteRecordFileFromFilePicker}
accept=".json"
>
Select File Manually…
Select Export Manually…
</FileInputButton>
<Button onPress={onClose}>Cancel</Button>
</React.Fragment>
Expand Down
6 changes: 3 additions & 3 deletions apps/admin/frontend/src/screens/tally_screen.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ test('with no data loaded', async () => {
await screen.findByRole('heading', {
name: 'Cast Vote Record (CVR) Management',
});
await screen.findByText('No CVR files loaded.');
expect(screen.getButton('Load CVR Files')).toBeEnabled();
expect(screen.getButton('Remove CVR Files')).toBeDisabled();
await screen.findByText('No CVRs loaded.');
expect(screen.getButton('Load CVRs')).toBeEnabled();
expect(screen.getButton('Remove CVRs')).toBeDisabled();
expect(screen.getButton('Add Manually Entered Results')).toBeEnabled();
expect(screen.getButton('Remove Manually Entered Results')).toBeDisabled();
});
Loading

0 comments on commit dc86724

Please sign in to comment.