-
Notifications
You must be signed in to change notification settings - Fork 357
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: run bulk action interstitial component (#9390)
Co-authored-by: Keita Fish <[email protected]>
- Loading branch information
1 parent
f3c4c65
commit 5b9be44
Showing
10 changed files
with
653 additions
and
25 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
216 changes: 216 additions & 0 deletions
216
webui/react/src/components/RunFilterInterstitialModalComponent.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,216 @@ | ||
import { act, render, screen } from '@testing-library/react'; | ||
import userEvent from '@testing-library/user-event'; | ||
import UIProvider, { DefaultTheme } from 'hew/Theme'; | ||
import { Ref } from 'react'; | ||
|
||
import { FilterFormSetWithoutId, FormField } from 'components/FilterForm/components/type'; | ||
import RunFilterInterstitialModalComponent, { | ||
CloseReason, | ||
ControlledModalRef, | ||
Props, | ||
} from 'components/RunFilterInterstitialModalComponent'; | ||
import { ThemeProvider } from 'components/ThemeProvider'; | ||
|
||
vi.mock('services/api', async () => ({ | ||
...(await vi.importActual<typeof import('services/api')>('services/api')), | ||
searchRuns: vi.fn(() => | ||
Promise.resolve({ | ||
pagination: { | ||
total: 0, | ||
}, | ||
}), | ||
), | ||
})); | ||
|
||
const { searchRuns } = await import('services/api'); | ||
const searchRunsMock = vi.mocked(searchRuns); | ||
|
||
const emptyFilterFormSetWithoutId: FilterFormSetWithoutId = { | ||
filterGroup: { | ||
children: [], | ||
conjunction: 'or', | ||
kind: 'group', | ||
}, | ||
showArchived: false, | ||
}; | ||
|
||
const setupTest = (props: Partial<Props> = {}) => { | ||
const ref: Ref<ControlledModalRef> = { current: null }; | ||
userEvent.setup(); | ||
|
||
render( | ||
<UIProvider theme={DefaultTheme.Light}> | ||
<ThemeProvider> | ||
<RunFilterInterstitialModalComponent | ||
filterFormSet={emptyFilterFormSetWithoutId} | ||
projectId={1} | ||
selection={{ selections: [], type: 'ONLY_IN' }} | ||
{...props} | ||
ref={ref} | ||
/> | ||
</ThemeProvider> | ||
</UIProvider>, | ||
); | ||
|
||
return { | ||
ref, | ||
}; | ||
}; | ||
|
||
describe('RunFilterInterstitialModalComponent', () => { | ||
beforeEach(() => { | ||
searchRunsMock.mockRestore(); | ||
}); | ||
|
||
it('does not call server until opened', () => { | ||
const { ref } = setupTest(); | ||
|
||
expect(searchRunsMock).not.toBeCalled(); | ||
act(() => { | ||
ref.current?.open(); | ||
}); | ||
expect(searchRunsMock).toBeCalled(); | ||
}); | ||
|
||
it('calls server with filter describing filter selection', () => { | ||
const expectedFilterGroup: FilterFormSetWithoutId['filterGroup'] = { | ||
children: [ | ||
{ | ||
columnName: 'experimentName', | ||
kind: 'field', | ||
location: 'LOCATION_TYPE_RUN', | ||
operator: 'contains', | ||
type: 'COLUMN_TYPE_TEXT', | ||
value: 'foo', | ||
}, | ||
], | ||
conjunction: 'and', | ||
kind: 'group', | ||
}; | ||
const expectedExclusions = [1, 2, 3]; | ||
const { ref } = setupTest({ | ||
filterFormSet: { | ||
filterGroup: expectedFilterGroup, | ||
showArchived: true, | ||
}, | ||
selection: { | ||
exclusions: expectedExclusions, | ||
type: 'ALL_EXCEPT', | ||
}, | ||
}); | ||
act(() => { | ||
ref.current?.open(); | ||
}); | ||
|
||
expect(searchRunsMock).toBeCalled(); | ||
|
||
const { lastCall } = vi.mocked(searchRuns).mock; | ||
const filterFormSetString = lastCall?.[0].filter; | ||
expect(filterFormSetString).toBeDefined(); | ||
const filterFormSet = JSON.parse(filterFormSetString || ''); | ||
|
||
// TODO: is there a better way to test this expectation? | ||
expect(filterFormSet.showArchived).toBeTruthy(); | ||
const [filterGroup, idFilterGroup] = filterFormSet.filterGroup.children?.[0].children || []; | ||
expect(filterGroup).toEqual(expectedFilterGroup); | ||
|
||
const idFilters = idFilterGroup.children; | ||
expect(idFilters.every((f: FormField) => f.operator === '!=')).toBeTruthy(); | ||
expect(idFilters.map((f: FormField) => f.value)).toEqual(expectedExclusions); | ||
}); | ||
|
||
it('calls server with filter describing visual selection', () => { | ||
const expectedSelection = [1, 2, 3]; | ||
const { ref } = setupTest({ | ||
selection: { | ||
selections: expectedSelection, | ||
type: 'ONLY_IN', | ||
}, | ||
}); | ||
act(() => { | ||
ref.current?.open(); | ||
}); | ||
|
||
expect(searchRunsMock).toBeCalled(); | ||
|
||
const { lastCall } = vi.mocked(searchRuns).mock; | ||
const filterFormSetString = lastCall?.[0].filter; | ||
expect(filterFormSetString).toBeDefined(); | ||
const filterFormSet = JSON.parse(filterFormSetString || ''); | ||
|
||
expect(filterFormSet.showArchived).toBe(false); | ||
const idFilters = filterFormSet.filterGroup.children?.[0].children || []; | ||
expect(idFilters.every((f: FormField) => f.operator === '=')).toBe(true); | ||
expect(idFilters.map((f: FormField) => f.value)).toEqual(expectedSelection); | ||
}); | ||
|
||
it('cancels request when modal is closed via close button', async () => { | ||
searchRunsMock.mockImplementation((_params, options) => { | ||
return new Promise((_resolve, reject) => { | ||
options?.signal?.addEventListener('abort', () => { | ||
reject(); | ||
}); | ||
}); | ||
}); | ||
const { ref } = setupTest(); | ||
// explicit type here because typescript can't infer that the act function | ||
// runs imperatively. | ||
let lifecycle: Promise<CloseReason> | undefined; | ||
// we don't await the act because we need the render pipeline to flush | ||
// before we get the close reason back | ||
act(() => { | ||
lifecycle = ref.current?.open(); | ||
}); | ||
const closeButton = await screen.findByLabelText('Close'); | ||
await userEvent.click(closeButton); | ||
const closeReason = await lifecycle; | ||
expect(closeReason).toBe('close'); | ||
}); | ||
|
||
it('closes modal with has_search_runs when it has runs', async () => { | ||
searchRunsMock.mockImplementation(() => | ||
Promise.resolve({ | ||
pagination: { | ||
total: 1, | ||
}, | ||
runs: [], | ||
}), | ||
); | ||
const { ref } = setupTest(); | ||
let lifecycle: Promise<CloseReason> | undefined; | ||
act(() => { | ||
lifecycle = ref.current?.open(); | ||
}); | ||
const closeReason = await lifecycle; | ||
expect(closeReason).toBe('has_search_runs'); | ||
}); | ||
|
||
it('closes modal with no_search_runs when it lacks runs', async () => { | ||
searchRunsMock.mockImplementation(() => | ||
Promise.resolve({ | ||
pagination: { | ||
total: 0, | ||
}, | ||
runs: [], | ||
}), | ||
); | ||
const { ref } = setupTest(); | ||
let lifecycle: Promise<CloseReason> | undefined; | ||
act(() => { | ||
lifecycle = ref.current?.open(); | ||
}); | ||
const closeReason = await lifecycle; | ||
expect(closeReason).toBe('no_search_runs'); | ||
}); | ||
|
||
it('closes modal with failed when request errors outside of aborts', async () => { | ||
searchRunsMock.mockImplementation(() => Promise.reject(new Error('uh oh!'))); | ||
const { ref } = setupTest(); | ||
let lifecycle: Promise<CloseReason> | undefined; | ||
act(() => { | ||
lifecycle = ref.current?.open(); | ||
}); | ||
const closeReason = await lifecycle; | ||
expect(closeReason).toBe('failed'); | ||
}); | ||
}); |
168 changes: 168 additions & 0 deletions
168
webui/react/src/components/RunFilterInterstitialModalComponent.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,168 @@ | ||
import { useModal } from 'hew/Modal'; | ||
import { Failed, NotLoaded } from 'hew/utils/loadable'; | ||
import { forwardRef, useCallback, useImperativeHandle, useRef, useState } from 'react'; | ||
|
||
import { FilterFormSetWithoutId, Operator } from 'components/FilterForm/components/type'; | ||
import InterstitialModalComponent, { | ||
onInterstitialCloseActionType, | ||
} from 'components/InterstitialModalComponent'; | ||
import { SelectionType } from 'components/Searches/Searches.settings'; | ||
import { useAsync } from 'hooks/useAsync'; | ||
import { searchRuns } from 'services/api'; | ||
import { DetError } from 'utils/error'; | ||
import mergeAbortControllers from 'utils/mergeAbortControllers'; | ||
import { observable } from 'utils/observable'; | ||
|
||
export type CloseReason = 'has_search_runs' | 'no_search_runs' | 'failed' | 'close' | 'manual'; | ||
|
||
export interface Props { | ||
projectId?: number; | ||
selection: SelectionType; | ||
filterFormSet: FilterFormSetWithoutId; | ||
} | ||
|
||
export interface ControlledModalRef { | ||
open: () => Promise<CloseReason>; | ||
close: (reason?: CloseReason) => void; | ||
} | ||
|
||
/** | ||
* Modal component for checking selections for runs that are part of a search. | ||
* is essentially a single purpose interstitial modal component. Because it | ||
* wraps a modal and the intended use is within a user flow, this component does | ||
* not use the `useModal` hook. instead, it exposes control via ref. the `open` | ||
* method of the ref returns a promise that resolves when the modal is closed | ||
* with the reason why the modal closed. | ||
* | ||
*/ | ||
export const RunFilterInterstitialModalComponent = forwardRef<ControlledModalRef, Props>( | ||
({ projectId, selection, filterFormSet }: Props, ref): JSX.Element => { | ||
const InterstitialModal = useModal(InterstitialModalComponent); | ||
const [isOpen, setIsOpen] = useState<boolean>(false); | ||
const closeController = useRef(new AbortController()); | ||
const lifecycleObservable = useRef(observable<CloseReason | null>(null)); | ||
|
||
const { close: internalClose, open: internalOpen } = InterstitialModal; | ||
|
||
const open = async () => { | ||
internalOpen(); | ||
setIsOpen(true); | ||
const closeReason = await lifecycleObservable.current.toPromise(); | ||
if (closeReason === null) { | ||
// this promise should never reject -- toPromise only resolves when the | ||
// value changes, and no code sets the observavble to null | ||
return Promise.reject(); | ||
} | ||
return closeReason; | ||
}; | ||
|
||
const close = useCallback( | ||
(reason: CloseReason = 'manual') => { | ||
setIsOpen(false); | ||
// encourage render with isOpen to false before closing to prevent | ||
// firing onCloseAction twice | ||
setTimeout(() => internalClose('close'), 0); | ||
closeController.current.abort(); | ||
closeController.current = new AbortController(); | ||
lifecycleObservable.current.set(reason); | ||
lifecycleObservable.current = observable(null); | ||
}, | ||
[internalClose], | ||
); | ||
|
||
useImperativeHandle(ref, () => ({ close, open })); | ||
|
||
const selectionHasSearchRuns = useAsync( | ||
async (canceler) => { | ||
if (!isOpen) return NotLoaded; | ||
const mergedCanceler = mergeAbortControllers(canceler, closeController.current); | ||
const idToFilter = (operator: Operator, id: number) => | ||
({ | ||
columnName: 'id', | ||
kind: 'field', | ||
location: 'LOCATION_TYPE_RUN', | ||
operator, | ||
type: 'COLUMN_TYPE_NUMBER', | ||
value: id, | ||
}) as const; | ||
const filterGroup: FilterFormSetWithoutId['filterGroup'] = | ||
selection.type === 'ALL_EXCEPT' | ||
? { | ||
children: [ | ||
filterFormSet.filterGroup, | ||
{ | ||
children: selection.exclusions.map(idToFilter.bind(this, '!=')), | ||
conjunction: 'and', | ||
kind: 'group', | ||
}, | ||
], | ||
conjunction: 'and', | ||
kind: 'group', | ||
} | ||
: { | ||
children: selection.selections.map(idToFilter.bind(this, '=')), | ||
conjunction: 'or', | ||
kind: 'group', | ||
}; | ||
const filter: FilterFormSetWithoutId = { | ||
...filterFormSet, | ||
filterGroup: { | ||
children: [ | ||
filterGroup, | ||
{ | ||
columnName: 'numTrials', | ||
kind: 'field', | ||
location: 'LOCATION_TYPE_EXPERIMENT', | ||
operator: '>', | ||
type: 'COLUMN_TYPE_NUMBER', | ||
value: 1, | ||
} as const, | ||
], | ||
conjunction: 'and', | ||
kind: 'group', | ||
}, | ||
}; | ||
try { | ||
const results = await searchRuns( | ||
{ | ||
filter: JSON.stringify(filter), | ||
limit: 0, | ||
projectId, | ||
}, | ||
{ signal: mergedCanceler.signal }, | ||
); | ||
|
||
return (results.pagination.total || 0) > 0; | ||
} catch (e) { | ||
if (!mergedCanceler.signal.aborted) { | ||
return Failed(e instanceof Error ? e : new DetError(e)); | ||
} | ||
return NotLoaded; | ||
} | ||
}, | ||
[selection, filterFormSet, projectId, isOpen], | ||
); | ||
|
||
const interstitialClose: onInterstitialCloseActionType = useCallback( | ||
(reason) => { | ||
if (reason === 'ok') { | ||
return selectionHasSearchRuns.forEach((bool) => { | ||
const fixedReason = bool ? 'has_search_runs' : 'no_search_runs'; | ||
close(fixedReason); | ||
}); | ||
} | ||
close(reason); | ||
}, | ||
[close, selectionHasSearchRuns], | ||
); | ||
|
||
return ( | ||
<InterstitialModal.Component | ||
loadableData={selectionHasSearchRuns} | ||
onCloseAction={interstitialClose} | ||
/> | ||
); | ||
}, | ||
); | ||
|
||
export default RunFilterInterstitialModalComponent; |
Oops, something went wrong.