Migrate code monitoring <Switch/> to RR6 (#47176)

This commit is contained in:
Philipp Spiess 2023-01-31 15:48:33 +01:00 committed by GitHub
parent 8f7ea8037f
commit 49f6f2bca2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 126 additions and 108 deletions

View File

@ -1,6 +1,6 @@
import { screen, waitFor } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import * as H from 'history'
import { Route, Routes } from 'react-router-dom-v5-compat'
import { NEVER, of } from 'rxjs'
import sinon from 'sinon'
@ -22,14 +22,11 @@ describe('CreateCodeMonitorPage', () => {
siteAdmin: true,
} as AuthenticatedUser
const history = H.createMemoryHistory()
const props = {
location: history.location,
authenticatedUser: mockUser,
breadcrumbs: [{ depth: 0, breadcrumb: null }],
setBreadcrumb: sinon.spy(),
useBreadcrumb: sinon.spy(),
history,
deleteCodeMonitor: sinon.spy((id: string) => NEVER),
createCodeMonitor: sinon.spy((monitor: CreateCodeMonitorVariables) =>
of({ description: mockCodeMonitor.node.description })
@ -56,8 +53,13 @@ describe('CreateCodeMonitorPage', () => {
renderWithBrandedContext(
<MockedTestProvider>
<CreateCodeMonitorPage {...props} location={{ ...history.location, search }} />
</MockedTestProvider>
<Routes>
<Route path="/code-monitoring/new" element={<CreateCodeMonitorPage {...props} />} />
</Routes>
</MockedTestProvider>,
{
route: '/code-monitoring/new?' + search,
}
)
const nameInput = screen.getByTestId('name-input')
userEvent.type(nameInput, 'Test updated')
@ -81,8 +83,13 @@ describe('CreateCodeMonitorPage', () => {
renderWithBrandedContext(
<MockedTestProvider>
<CreateCodeMonitorPage {...props} location={{ ...history.location, search }} />
</MockedTestProvider>
<Routes>
<Route path="/code-monitoring/new" element={<CreateCodeMonitorPage {...props} />} />
</Routes>
</MockedTestProvider>,
{
route: '/code-monitoring/new?' + search,
}
)
const nameInput = screen.getByTestId('name-input')
userEvent.type(nameInput, 'Test updated')
@ -103,8 +110,11 @@ describe('CreateCodeMonitorPage', () => {
test('Actions area button is disabled while trigger is incomplete', () => {
renderWithBrandedContext(
<MockedTestProvider>
<CreateCodeMonitorPage {...props} />
</MockedTestProvider>
<Routes>
<Route path="/code-monitoring/new" element={<CreateCodeMonitorPage {...props} />} />
</Routes>
</MockedTestProvider>,
{ route: '/code-monitoring/new' }
)
const actionButton = screen.getByTestId('form-action-toggle-email')
assertAriaDisabled(actionButton)

View File

@ -1,7 +1,7 @@
import React, { useCallback, useEffect, useMemo } from 'react'
import { VisuallyHidden } from '@reach/visually-hidden'
import * as H from 'history'
import { useLocation } from 'react-router-dom-v5-compat'
import { Observable } from 'rxjs'
import { ThemeProps } from '@sourcegraph/shared/src/theme'
@ -19,8 +19,6 @@ import { createCodeMonitor as _createCodeMonitor } from './backend'
import { CodeMonitorForm } from './components/CodeMonitorForm'
interface CreateCodeMonitorPageProps extends ThemeProps {
location: H.Location
history: H.History
authenticatedUser: AuthenticatedUser
createCodeMonitor?: typeof _createCodeMonitor
@ -30,14 +28,9 @@ interface CreateCodeMonitorPageProps extends ThemeProps {
const AuthenticatedCreateCodeMonitorPage: React.FunctionComponent<
React.PropsWithChildren<CreateCodeMonitorPageProps>
> = ({
authenticatedUser,
history,
location,
createCodeMonitor = _createCodeMonitor,
isLightTheme,
isSourcegraphDotCom,
}) => {
> = ({ authenticatedUser, createCodeMonitor = _createCodeMonitor, isLightTheme, isSourcegraphDotCom }) => {
const location = useLocation()
const triggerQuery = useMemo(
() => new URLSearchParams(location.search).get('trigger-query') ?? undefined,
[location.search]
@ -98,8 +91,6 @@ const AuthenticatedCreateCodeMonitorPage: React.FunctionComponent<
</PageHeader.Heading>
</PageHeader>
<CodeMonitorForm
history={history}
location={location}
authenticatedUser={authenticatedUser}
onSubmit={createMonitorRequest}
triggerQuery={triggerQuery}

View File

@ -1,7 +1,7 @@
import { screen } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import * as H from 'history'
import { NEVER, of } from 'rxjs'
import { Route, Routes } from 'react-router-dom-v5-compat/'
import { of } from 'rxjs'
import sinon from 'sinon'
import { MockedTestProvider } from '@sourcegraph/shared/src/testing/apollo'
@ -30,11 +30,7 @@ describe('ManageCodeMonitorPage', () => {
window.context = origContext
})
const history = H.createMemoryHistory()
history.location.pathname = '/code-monitoring/test-monitor-id'
const props = {
history,
location: history.location,
authenticatedUser: mockUser,
breadcrumbs: [{ depth: 0, breadcrumb: null }],
setBreadcrumb: sinon.spy(),
@ -47,14 +43,8 @@ describe('ManageCodeMonitorPage', () => {
) => of(mockCodeMonitorFields)
),
fetchCodeMonitor: sinon.spy((id: string) => of(mockCodeMonitor as FetchCodeMonitorResult)),
match: {
params: { id: 'test-id' },
isExact: true,
path: history.location.pathname,
url: 'https://sourcegraph.com',
},
toggleCodeMonitorEnabled: sinon.spy((id: string, enabled: boolean) => of({ id: 'test', enabled: true })),
deleteCodeMonitor: sinon.spy((id: string) => NEVER),
deleteCodeMonitor: sinon.spy((id: string) => of(undefined)),
isLightTheme: false,
isSourcegraphDotCom: false,
}
@ -62,8 +52,11 @@ describe('ManageCodeMonitorPage', () => {
test('Form is pre-loaded with code monitor data', () => {
renderWithBrandedContext(
<MockedTestProvider>
<ManageCodeMonitorPage {...props} />
</MockedTestProvider>
<Routes>
<Route path="/code-monitoring/:id" element={<ManageCodeMonitorPage {...props} />} />
</Routes>
</MockedTestProvider>,
{ route: '/code-monitoring/test-monitor-id' }
)
expect(props.fetchCodeMonitor.calledOnce).toBe(true)
@ -81,8 +74,11 @@ describe('ManageCodeMonitorPage', () => {
test('Updating the form executes the update request', () => {
renderWithBrandedContext(
<MockedTestProvider>
<ManageCodeMonitorPage {...props} />
</MockedTestProvider>
<Routes>
<Route path="/code-monitoring/:id" element={<ManageCodeMonitorPage {...props} />} />
</Routes>
</MockedTestProvider>,
{ route: '/code-monitoring/test-monitor-id' }
)
const nameInput = screen.getByTestId('name-input')
expect(nameInput).toHaveValue('Test code monitor')
@ -95,7 +91,7 @@ describe('ManageCodeMonitorPage', () => {
sinon.assert.calledWith(
props.updateCodeMonitor,
{
id: 'test-id',
id: 'test-monitor-id',
update: { namespace: 'userID', description: 'Test updated', enabled: true },
},
{ id: 'test-0', update: { query: 'test' } },
@ -130,8 +126,11 @@ describe('ManageCodeMonitorPage', () => {
test('Clicking Edit in the trigger area opens the query form', () => {
renderWithBrandedContext(
<MockedTestProvider>
<ManageCodeMonitorPage {...props} />
</MockedTestProvider>
<Routes>
<Route path="/code-monitoring/:id" element={<ManageCodeMonitorPage {...props} />} />
</Routes>
</MockedTestProvider>,
{ route: '/code-monitoring/test-monitor-id' }
)
expect(screen.queryByTestId('trigger-query-edit')).not.toBeInTheDocument()
userEvent.click(screen.getByTestId('trigger-button'))
@ -141,8 +140,11 @@ describe('ManageCodeMonitorPage', () => {
test('Clicking Edit in the action area opens the action form', () => {
renderWithBrandedContext(
<MockedTestProvider>
<ManageCodeMonitorPage {...props} />
</MockedTestProvider>
<Routes>
<Route path="/code-monitoring/:id" element={<ManageCodeMonitorPage {...props} />} />
</Routes>
</MockedTestProvider>,
{ route: '/code-monitoring/test-monitor-id' }
)
expect(screen.queryByTestId('action-form-email')).not.toBeInTheDocument()
const editTrigger = screen.getByTestId('form-action-toggle-email')
@ -153,8 +155,11 @@ describe('ManageCodeMonitorPage', () => {
test('Save button is disabled when no changes have been made, enabled when changes have been made', () => {
renderWithBrandedContext(
<MockedTestProvider>
<ManageCodeMonitorPage {...props} />
</MockedTestProvider>
<Routes>
<Route path="/code-monitoring/:id" element={<ManageCodeMonitorPage {...props} />} />
</Routes>
</MockedTestProvider>,
{ route: '/code-monitoring/test-monitor-id' }
)
const submitButton = screen.getByTestId('submit-monitor')
assertAriaDisabled(submitButton)
@ -167,8 +172,11 @@ describe('ManageCodeMonitorPage', () => {
test('Cancelling after changes have been made shows confirmation prompt', () => {
renderWithBrandedContext(
<MockedTestProvider>
<ManageCodeMonitorPage {...props} />
</MockedTestProvider>
<Routes>
<Route path="/code-monitoring/:id" element={<ManageCodeMonitorPage {...props} />} />
</Routes>
</MockedTestProvider>,
{ route: '/code-monitoring/test-monitor-id' }
)
const confirmStub = sinon.stub(window, 'confirm')
@ -182,8 +190,11 @@ describe('ManageCodeMonitorPage', () => {
test('Cancelling without any changes made does not show confirmation prompt', () => {
renderWithBrandedContext(
<MockedTestProvider>
<ManageCodeMonitorPage {...props} />
</MockedTestProvider>
<Routes>
<Route path="/code-monitoring/:id" element={<ManageCodeMonitorPage {...props} />} />
</Routes>
</MockedTestProvider>,
{ route: '/code-monitoring/test-monitor-id' }
)
const confirmStub = sinon.stub(window, 'confirm')
userEvent.click(screen.getByTestId('cancel-monitor'))
@ -193,10 +204,18 @@ describe('ManageCodeMonitorPage', () => {
})
test('Clicking delete code monitor opens deletion confirmation modal', () => {
let currentPathname = ''
renderWithBrandedContext(
<MockedTestProvider>
<ManageCodeMonitorPage {...props} />
</MockedTestProvider>
<Routes>
<Route path="/code-monitoring/:id" element={<ManageCodeMonitorPage {...props} />} />
<Route path="/code-monitoring" element={null} />
</Routes>
</MockedTestProvider>,
{
route: '/code-monitoring/test-monitor-id',
onLocationChange: location => (currentPathname = location.pathname),
}
)
userEvent.click(screen.getByTestId('delete-monitor'))
expect(screen.getByTestId('delete-modal')).toBeInTheDocument()
@ -206,6 +225,6 @@ describe('ManageCodeMonitorPage', () => {
userEvent.click(confirmDeleteButton)
sinon.assert.calledOnce(props.deleteCodeMonitor)
expect(props.history.location.pathname).toEqual('/code-monitoring')
expect(currentPathname).toEqual('/code-monitoring')
})
})

View File

@ -1,13 +1,11 @@
import React, { useEffect } from 'react'
import { VisuallyHidden } from '@reach/visually-hidden'
import * as H from 'history'
import { RouteComponentProps } from 'react-router'
import { useParams } from 'react-router-dom-v5-compat'
import { Observable } from 'rxjs'
import { startWith, catchError, tap } from 'rxjs/operators'
import { asError, isErrorLike } from '@sourcegraph/common'
import { Scalars } from '@sourcegraph/shared/src/graphql-operations'
import { ThemeProps } from '@sourcegraph/shared/src/theme'
import { PageHeader, Link, LoadingSpinner, useObservable } from '@sourcegraph/wildcard'
@ -26,10 +24,8 @@ import {
} from './backend'
import { CodeMonitorForm } from './components/CodeMonitorForm'
interface ManageCodeMonitorPageProps extends RouteComponentProps<{ id: Scalars['ID'] }>, ThemeProps {
interface ManageCodeMonitorPageProps extends ThemeProps {
authenticatedUser: AuthenticatedUser
location: H.Location
history: H.History
fetchCodeMonitor?: typeof _fetchCodeMonitor
updateCodeMonitor?: typeof _updateCodeMonitor
@ -42,9 +38,6 @@ const AuthenticatedManageCodeMonitorPage: React.FunctionComponent<
React.PropsWithChildren<ManageCodeMonitorPageProps>
> = ({
authenticatedUser,
history,
location,
match,
fetchCodeMonitor = _fetchCodeMonitor,
updateCodeMonitor = _updateCodeMonitor,
deleteCodeMonitor = _deleteCodeMonitor,
@ -55,6 +48,8 @@ const AuthenticatedManageCodeMonitorPage: React.FunctionComponent<
useEffect(() => eventLogger.logPageView('ManageCodeMonitorPage'), [])
const { id } = useParams()
const [codeMonitorState, setCodeMonitorState] = React.useState<CodeMonitorFields>({
id: '',
description: '',
@ -66,7 +61,7 @@ const AuthenticatedManageCodeMonitorPage: React.FunctionComponent<
const codeMonitorOrError = useObservable(
React.useMemo(
() =>
fetchCodeMonitor(match.params.id).pipe(
fetchCodeMonitor(id!).pipe(
tap(monitor => {
if (monitor.node !== null && monitor.node.__typename === 'Monitor') {
setCodeMonitorState(monitor.node)
@ -75,7 +70,7 @@ const AuthenticatedManageCodeMonitorPage: React.FunctionComponent<
startWith(LOADING),
catchError(error => [asError(error)])
),
[match.params.id, fetchCodeMonitor]
[id, fetchCodeMonitor]
)
)
@ -84,7 +79,7 @@ const AuthenticatedManageCodeMonitorPage: React.FunctionComponent<
eventLogger.log('ManageCodeMonitorFormSubmitted')
return updateCodeMonitor(
{
id: match.params.id,
id: id!,
update: {
namespace: authenticatedUser.id,
description: codeMonitor.description,
@ -95,7 +90,7 @@ const AuthenticatedManageCodeMonitorPage: React.FunctionComponent<
convertActionsForUpdate(codeMonitor.actions.nodes, authenticatedUser.id)
)
},
[authenticatedUser.id, match.params.id, updateCodeMonitor]
[authenticatedUser.id, id, updateCodeMonitor]
)
const deleteMonitorRequest = React.useCallback(
@ -133,8 +128,6 @@ const AuthenticatedManageCodeMonitorPage: React.FunctionComponent<
{codeMonitorOrError && !isErrorLike(codeMonitorOrError) && codeMonitorOrError !== 'loading' && (
<>
<CodeMonitorForm
history={history}
location={location}
authenticatedUser={authenticatedUser}
deleteCodeMonitor={deleteMonitorRequest}
onSubmit={updateMonitorRequest}

View File

@ -1,5 +1,4 @@
import { fireEvent, getByRole, screen } from '@testing-library/react'
import { createMemoryHistory, createLocation } from 'history'
import { NEVER } from 'rxjs'
import { MockedTestProvider } from '@sourcegraph/shared/src/testing/apollo'
@ -11,8 +10,6 @@ import { mockAuthenticatedUser, mockCodeMonitorFields } from '../testing/util'
import { CodeMonitorForm, CodeMonitorFormProps } from './CodeMonitorForm'
const PROPS: CodeMonitorFormProps = {
history: createMemoryHistory(),
location: createLocation('/code-monitoring/new'),
onSubmit: () => NEVER,
submitButtonLabel: '',
authenticatedUser: mockAuthenticatedUser,

View File

@ -1,8 +1,8 @@
import React, { useCallback, useMemo, useState } from 'react'
import classNames from 'classnames'
import * as H from 'history'
import { isEqual } from 'lodash'
import { useNavigate } from 'react-router-dom-v5-compat'
import { Observable } from 'rxjs'
import { mergeMap, startWith, catchError, tap, filter } from 'rxjs/operators'
@ -22,8 +22,6 @@ import { FormTriggerArea } from './FormTriggerArea'
import styles from './CodeMonitorForm.module.scss'
export interface CodeMonitorFormProps extends ThemeProps {
history: H.History
location: H.Location
authenticatedUser: AuthenticatedUser
/**
* A function that takes in a code monitor and emits an Observable with all or some
@ -54,7 +52,6 @@ interface FormCompletionSteps {
export const CodeMonitorForm: React.FunctionComponent<React.PropsWithChildren<CodeMonitorFormProps>> = ({
authenticatedUser,
onSubmit,
history,
submitButtonLabel,
codeMonitor,
showDeleteButton,
@ -66,6 +63,8 @@ export const CodeMonitorForm: React.FunctionComponent<React.PropsWithChildren<Co
}) => {
const LOADING = 'loading' as const
const navigate = useNavigate()
const [currentCodeMonitorState, setCodeMonitor] = useState<CodeMonitorFields>(
codeMonitor ?? {
id: '',
@ -119,13 +118,13 @@ export const CodeMonitorForm: React.FunctionComponent<React.PropsWithChildren<Co
catchError(error => [asError(error)]),
tap(successOrError => {
if (!isErrorLike(successOrError) && successOrError !== LOADING) {
history.push('/code-monitoring')
navigate('/code-monitoring')
}
})
)
)
),
[onSubmit, currentCodeMonitorState, history, formCompletion]
[onSubmit, currentCodeMonitorState, navigate, formCompletion]
)
)
@ -141,12 +140,12 @@ export const CodeMonitorForm: React.FunctionComponent<React.PropsWithChildren<Co
const onCancel = useCallback(() => {
if (hasChangedFields) {
if (window.confirm('Leave page? All unsaved changes will be lost.')) {
history.push('/code-monitoring')
navigate('/code-monitoring')
}
} else {
history.push('/code-monitoring')
navigate('/code-monitoring')
}
}, [history, hasChangedFields])
}, [navigate, hasChangedFields])
const [showDeleteModal, setShowDeleteModal] = useState(false)
@ -299,7 +298,6 @@ export const CodeMonitorForm: React.FunctionComponent<React.PropsWithChildren<Co
<DeleteMonitorModal
isOpen={showDeleteModal}
deleteCodeMonitor={deleteCodeMonitor}
history={history}
codeMonitor={codeMonitor}
toggleDeleteModal={toggleDeleteModal}
/>

View File

@ -1,5 +1,6 @@
import React, { useCallback } from 'react'
import { useNavigate } from 'react-router-dom-v5-compat'
import { Observable, throwError } from 'rxjs'
import { mergeMap, startWith, tap, catchError } from 'rxjs/operators'
@ -8,20 +9,20 @@ import { Button, LoadingSpinner, useEventObservable, Modal, Alert, H3, Text } fr
import { CodeMonitorFormProps } from './CodeMonitorForm'
interface DeleteModalProps extends Pick<CodeMonitorFormProps, 'history' | 'codeMonitor'> {
interface DeleteModalProps extends Pick<CodeMonitorFormProps, 'codeMonitor'> {
isOpen: boolean
toggleDeleteModal: () => void
deleteCodeMonitor: (id: string) => Observable<void>
}
export const DeleteMonitorModal: React.FunctionComponent<React.PropsWithChildren<DeleteModalProps>> = ({
history,
isOpen,
deleteCodeMonitor,
toggleDeleteModal,
codeMonitor,
}) => {
const LOADING = 'loading' as const
const navigate = useNavigate()
const deleteLabelId = 'deleteCodeMonitor'
@ -33,7 +34,7 @@ export const DeleteMonitorModal: React.FunctionComponent<React.PropsWithChildren
if (codeMonitor) {
return deleteCodeMonitor(codeMonitor.id).pipe(
tap(() => {
history.push('/code-monitoring')
navigate('/code-monitoring')
}),
startWith(LOADING),
catchError(error => [asError(error)])
@ -43,7 +44,7 @@ export const DeleteMonitorModal: React.FunctionComponent<React.PropsWithChildren
return throwError(new Error('Failed to delete: Code monitor ID not provided'))
})
),
[deleteCodeMonitor, history, codeMonitor]
[deleteCodeMonitor, navigate, codeMonitor]
)
)

View File

@ -1,6 +1,6 @@
import React from 'react'
import { Route, Switch } from 'react-router'
import { Routes, Route } from 'react-router-dom-v5-compat'
import { PlatformContextProps } from '@sourcegraph/shared/src/platform/context'
import { SettingsCascadeProps } from '@sourcegraph/shared/src/settings/settings'
@ -28,23 +28,11 @@ export const GlobalCodeMonitoringArea: React.FunctionComponent<React.PropsWithCh
}) => (
<div className="w-100">
<Page>
<Switch>
<Route
path="/code-monitoring"
render={props => <CodeMonitoringPage {...outerProps} {...props} />}
exact={true}
/>
<Route
path="/code-monitoring/new"
render={props => <CreateCodeMonitorPage {...outerProps} {...props} />}
exact={true}
/>
<Route
path="/code-monitoring/:id"
render={props => <ManageCodeMonitorPage {...outerProps} {...props} />}
exact={true}
/>
</Switch>
<Routes>
<Route path="" element={<CodeMonitoringPage {...outerProps} />} />
<Route path="new" element={<CreateCodeMonitorPage {...outerProps} />} />
<Route path=":id" element={<ManageCodeMonitorPage {...outerProps} />} />
</Routes>
</Page>
</div>
)

View File

@ -1,9 +1,10 @@
import { ReactNode } from 'react'
import { ReactNode, useEffect, useLayoutEffect, useRef } from 'react'
import { RenderResult, render } from '@testing-library/react'
import { MemoryHistory, createMemoryHistory } from 'history'
import * as H from 'history'
import { Router } from 'react-router-dom'
import { CompatRouter } from 'react-router-dom-v5-compat'
import { CompatRouter, useLocation } from 'react-router-dom-v5-compat'
import { WildcardThemeContext, WildcardTheme } from '../hooks/useWildcardTheme'
@ -14,6 +15,7 @@ export interface RenderWithBrandedContextResult extends RenderResult {
interface RenderWithBrandedContextOptions {
route?: string
history?: MemoryHistory<unknown>
onLocationChange?: (location: H.Location) => void
}
const wildcardTheme: WildcardTheme = {
@ -22,16 +24,35 @@ const wildcardTheme: WildcardTheme = {
export function renderWithBrandedContext(
children: ReactNode,
{ route = '/', history = createMemoryHistory({ initialEntries: [route] }) }: RenderWithBrandedContextOptions = {}
{
route = '/',
history = createMemoryHistory({ initialEntries: [route] }),
onLocationChange = (_location: H.Location) => {},
}: RenderWithBrandedContextOptions = {}
): RenderWithBrandedContextResult {
return {
...render(
<WildcardThemeContext.Provider value={wildcardTheme}>
<Router history={history}>
<CompatRouter>{children}</CompatRouter>
<CompatRouter>
{children}
<ExtractCurrentPathname onLocationChange={onLocationChange} />
</CompatRouter>
</Router>
</WildcardThemeContext.Provider>
),
history,
}
}
function ExtractCurrentPathname({ onLocationChange }: { onLocationChange: (location: H.Location) => void }): null {
const onLocationChangeRef = useRef(onLocationChange)
useLayoutEffect(() => {
onLocationChangeRef.current = onLocationChange
}, [onLocationChange])
const location = useLocation()
useEffect(() => {
onLocationChangeRef.current(location)
}, [location, onLocationChange])
return null
}