Skip to content

Commit

Permalink
chore(kiosk-browser): remove uses of non-trivial APIs
Browse files Browse the repository at this point in the history
Closes #4531

Removes all uses of the following `kiosk-browser` APIs:
- `captureScreenshot` + `saveAs` (replaced with `dev-dock/backend` API + `gnome-screenshot`)
- `showOpenDialog` (selecting election package to load in dev is removed)
- `showSaveDialog` ("Save As" in a few places is removed)

We still use `kiosk.quit` and `kiosk.log`, but these can be trivially reimplemented.
  • Loading branch information
eventualbuddha committed Nov 26, 2024
1 parent c4d9e0f commit f1813fb
Show file tree
Hide file tree
Showing 13 changed files with 33 additions and 370 deletions.
126 changes: 0 additions & 126 deletions apps/admin/frontend/src/components/save_backend_file_modal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,52 +56,6 @@ test('render no usb found screen when there is not a valid mounted usb drive', (
}
});

test('has development shortcut to export file without USB drive', async () => {
const mockShowSaveDialog = jest
.fn()
.mockResolvedValue({ filePath: '/user/batch-export.csv' });
kiosk.showSaveDialog = mockShowSaveDialog;

const originalEnv: NodeJS.ProcessEnv = { ...process.env };
process.env = {
...originalEnv,
NODE_ENV: 'development',
};

const saveFile = jest.fn().mockResolvedValue(ok());

renderInAppContext(
<SaveBackendFileModal
saveFileStatus="idle"
saveFile={saveFile}
saveFileResult={undefined}
resetSaveFileResult={jest.fn()}
onClose={jest.fn()}
defaultRelativePath="batch-export.csv"
fileTypeTitle="Batch Export"
fileType="batch export"
/>,
{
usbDriveStatus: mockUsbDriveStatus('no_drive'),
apiMock,
}
);

// TODO: remove when USB status comes from backend. currently, allows
// component to set the usb drive path in useEffect
await advancePromises();

userEvent.click(screen.getButton('Save As…'));
expect(mockShowSaveDialog).toHaveBeenCalledWith({
defaultPath: 'batch-export.csv',
});
await waitFor(() => {
expect(saveFile).toHaveBeenCalledWith({ path: '/user/batch-export.csv' });
});

process.env = originalEnv;
});

test('happy usb path - save to default location', async () => {
const saveFile = jest.fn().mockResolvedValue(ok());

Expand Down Expand Up @@ -134,47 +88,6 @@ test('happy usb path - save to default location', async () => {
});
});

test('happy usb path - save as', async () => {
const mockShowSaveDialog = jest.fn().mockResolvedValue({
filePath: 'test-mount-point/batch-export.csv',
});
kiosk.showSaveDialog = mockShowSaveDialog;

const saveFile = jest.fn().mockResolvedValue(ok());

renderInAppContext(
<SaveBackendFileModal
saveFileStatus="idle"
saveFile={saveFile}
saveFileResult={undefined}
resetSaveFileResult={jest.fn()}
onClose={jest.fn()}
defaultRelativePath="exports/batch-export.csv"
fileTypeTitle="Batch Export"
fileType="batch export"
/>,
{
usbDriveStatus: mockUsbDriveStatus('mounted'),
apiMock,
}
);
await screen.findByText('Save Batch Export');

// TODO: remove when USB status comes from backend. currently, allows
// component to set the usb drive path in useEffect
await advancePromises();

userEvent.click(screen.getButton('Save As…'));
expect(mockShowSaveDialog).toHaveBeenCalledWith({
defaultPath: 'test-mount-point/batch-export.csv',
});
await waitFor(() => {
expect(saveFile).toHaveBeenCalledWith({
path: 'test-mount-point/batch-export.csv',
});
});
});

test('renders saving modal when mutation is loading', () => {
renderInAppContext(
<SaveBackendFileModal
Expand Down Expand Up @@ -256,42 +169,3 @@ test('shows error screen if saving file failed on backend', () => {
screen.getByText('Batch Export Not Saved');
screen.getByText('Failed to save batch export. Permission denied.');
});

test('can cancel save dialog', async () => {
const mockShowSaveDialog = jest.fn().mockResolvedValue({ canceled: true });
kiosk.showSaveDialog = mockShowSaveDialog;

const saveFile = jest.fn().mockResolvedValue(ok());

renderInAppContext(
<SaveBackendFileModal
saveFileStatus="idle"
saveFile={saveFile}
saveFileResult={undefined}
resetSaveFileResult={jest.fn()}
onClose={jest.fn()}
defaultRelativePath="exports/batch-export.csv"
fileTypeTitle="Batch Export"
fileType="batch export"
/>,
{
usbDriveStatus: mockUsbDriveStatus('mounted'),
apiMock,
}
);
await screen.findByText('Save Batch Export');
// TODO: remove when USB status comes from backend. currently, allows
// component to set the usb drive path in useEffect
await advancePromises();
userEvent.click(screen.getButton('Save As…'));
expect(mockShowSaveDialog).toHaveBeenCalledWith({
defaultPath: 'test-mount-point/batch-export.csv',
});

// because the save dialog is not part of the UI, we cannot wait for its disappearance,
// but we need to allow the save as button to settle
await advancePromises();

expect(saveFile).not.toHaveBeenCalled();
screen.getByText('Save Batch Export');
});
52 changes: 1 addition & 51 deletions apps/admin/frontend/src/components/save_backend_file_modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,31 +13,6 @@ import { MutationStatus } from '@tanstack/react-query';
import { AppContext } from '../contexts/app_context';
import { Loading } from './loading';

interface SaveAsButtonProps {
onSave: (location: string) => void;
disabled?: boolean;
options?: KioskBrowser.SaveDialogOptions;
}

function SaveAsButton({
onSave,
disabled,
options,
}: SaveAsButtonProps): JSX.Element {
async function useSaveDialog() {
assert(window.kiosk);
const { filePath } = await window.kiosk.showSaveDialog(options);
if (filePath) {
onSave(filePath);
}
}
return (
<Button onPress={useSaveDialog} disabled={disabled}>
Save As…
</Button>
);
}

export interface SaveBackendFileModalProps {
saveFileStatus: MutationStatus;
saveFile: (input: { path: string }) => void;
Expand Down Expand Up @@ -90,21 +65,7 @@ export function SaveBackendFileModal({
</P>
}
onOverlayClick={onClose}
actions={
<React.Fragment>
{window.kiosk && process.env.NODE_ENV === 'development' && (
<SaveAsButton
onSave={(path) => saveFile({ path })}
options={{
// Provide a file name, but allow the system dialog to use its
// default starting directory.
defaultPath: basename(defaultRelativePath),
}}
/>
)}
<Button onPress={onClose}>Cancel</Button>
</React.Fragment>
}
actions={<Button onPress={onClose}>Cancel</Button>}
/>
);
case 'mounted': {
Expand Down Expand Up @@ -135,17 +96,6 @@ export function SaveBackendFileModal({
Save
</Button>
<Button onPress={onClose}>Cancel</Button>
<SaveAsButton
onSave={(path) => saveFile({ path })}
options={{
// Provide a file name and default to the USB drive's root directory.
defaultPath: join(
usbDriveStatus.mountPoint,
basename(defaultRelativePath)
),
}}
disabled={!window.kiosk}
/>
</React.Fragment>
}
/>
Expand Down
23 changes: 0 additions & 23 deletions apps/admin/frontend/src/screens/unconfigured_screen.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import userEvent from '@testing-library/user-event';
import { mockKiosk } from '@votingworks/test-utils';
import { err } from '@votingworks/basics';
import { mockUsbDriveStatus } from '@votingworks/ui';
import { renderInAppContext } from '../../test/render_in_app_context';
Expand Down Expand Up @@ -40,7 +39,6 @@ test('handles no election packages on USB drive', async () => {

await screen.findByRole('heading', { name: 'Election' });
screen.getByText('No election packages found on the inserted USB drive.');
screen.getButton('Select Other File...');
});

test('configures from election packages on USB drive', async () => {
Expand Down Expand Up @@ -88,27 +86,6 @@ test('configures from election packages on USB drive', async () => {
await waitFor(() => apiMock.assertComplete());
});

test('configures from selected file', async () => {
const kiosk = mockKiosk();
window.kiosk = kiosk;
kiosk.showOpenDialog.mockResolvedValueOnce({
canceled: false,
filePaths: ['/path/to/election-package.zip'],
});

apiMock.expectListPotentialElectionPackagesOnUsbDrive([]);
renderInAppContext(<UnconfiguredScreen />, {
apiMock,
usbDriveStatus: mockUsbDriveStatus('mounted'),
});

await screen.findByRole('heading', { name: 'Election' });

apiMock.expectConfigure('/path/to/election-package.zip');
userEvent.click(screen.getButton('Select Other File...'));
await waitFor(() => apiMock.assertComplete());
});

test('shows configuration error', async () => {
apiMock.expectListPotentialElectionPackagesOnUsbDrive([
{
Expand Down
23 changes: 1 addition & 22 deletions apps/admin/frontend/src/screens/unconfigured_screen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React, { useContext } from 'react';
import styled from 'styled-components';
import { DateTime } from 'luxon';
import {
Button,
Card,
FullScreenMessage,
H2,
Expand All @@ -11,7 +10,7 @@ import {
UsbDriveImage,
} from '@votingworks/ui';
import type { FileSystemEntry } from '@votingworks/fs';
import { assertDefined, throwIllegalValue } from '@votingworks/basics';
import { throwIllegalValue } from '@votingworks/basics';
import { Loading } from '../components/loading';
import { NavigationScreen } from '../components/navigation_screen';
import { configure, listPotentialElectionPackagesOnUsbDrive } from '../api';
Expand All @@ -37,18 +36,6 @@ function SelectElectionPackage({
}): JSX.Element {
const configureMutation = configure.useMutation();

async function onSelectOtherFile() {
const dialogResult = await assertDefined(window.kiosk).showOpenDialog({
properties: ['openFile'],
filters: [{ name: '', extensions: ['zip', 'json'] }],
});
if (dialogResult.canceled) return;
const selectedPath = dialogResult.filePaths[0];
if (selectedPath) {
configureMutation.mutate({ electionFilePath: selectedPath });
}
}

const configureError = configureMutation.data?.err();

return (
Expand Down Expand Up @@ -111,14 +98,6 @@ function SelectElectionPackage({
</tbody>
</Table>
)}
<div>
<Button
disabled={configureMutation.isLoading}
onPress={onSelectOtherFile}
>
Select Other File...
</Button>
</div>
</div>
</React.Fragment>
);
Expand Down
80 changes: 0 additions & 80 deletions libs/@types/kiosk-browser/kiosk-browser.d.ts
Original file line number Diff line number Diff line change
@@ -1,87 +1,7 @@
declare namespace KioskBrowser {
export interface SaveAsOptions {
title?: string;
defaultPath?: string;
buttonLabel?: string;
filters?: FileFilter[];
}

// Copied from Electron.OpenDialogOptions
export interface OpenDialogOptions {
title?: string;
defaultPath?: string;
/**
* Custom label for the confirmation button, when left empty the default label will
* be used.
*/
buttonLabel?: string;
filters?: FileFilter[];
/**
* Contains which features the dialog should use. The following values are
* supported:
*/
properties?: Array<
'openFile' | 'openDirectory' | 'multiSelections' | 'showHiddenFiles'
>;
}

// Copied from Electron.SaveDialogOptions, with Linux relevant options only
export interface SaveDialogOptions {
/**
* The dialog title. Cannot be displayed on some `Linux` desktop environments.
*/
title?: string;
/**
* Absolute directory path, absolute file path, or file name to use by default.
*/
defaultPath?: string;
/**
* Custom label for the confirmation button, when left empty the default label will
* be used.
*/
buttonLabel?: string;
filters?: FileFilter[];
properties?: Array<'showHiddenFiles' | 'showOverwriteConfirmation'>;
}

export interface FileWriter {
/**
* Writes a chunk to the file. May be called multiple times. Data will be
* written in the order of calls to `write`.
*/
write(data: Uint8Array | string): Promise<void>;

/**
* Finishes writing to the file and closes it. Subsequent calls to `write`
* will fail. Resolves when the file is successfully closed.
*/
end(): Promise<void>;

filename: string;
}

export interface Kiosk {
log(message: string): Promise<void>;

quit(): void;

/**
* Opens a Save Dialog to allow the user to choose a destination for a file.
* Once chosen, resolves with a handle to the file to write data to it.
*/
saveAs(options?: SaveAsOptions): Promise<FileWriter | undefined>;

showOpenDialog(options?: OpenDialogOptions): Promise<{
canceled: boolean;
filePaths: string[];
}>;

showSaveDialog(options?: SaveDialogOptions): Promise<{
canceled: boolean;
filePath?: string;
}>;

captureScreenshot(): Promise<Buffer>;
}
}

Expand Down
Loading

0 comments on commit f1813fb

Please sign in to comment.