Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dropdowns to router route form for destinations and targets #2448

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion app/api/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ export const getUsePrefetchedApiQuery =
`Expected query to be prefetched.
Key: ${JSON.stringify(queryKey)}
Ensure the following:
• loader is running
• loader is called in routes.tsx and is running
charliepark marked this conversation as resolved.
Show resolved Hide resolved
• query matches in both the loader and the component
• request isn't erroring-out server-side (check the Networking tab)
• mock API endpoint is implemented in handlers.ts
Expand Down
67 changes: 53 additions & 14 deletions app/forms/vpc-router-route-common.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,19 @@

import type { UseFormReturn } from 'react-hook-form'

import type {
RouteDestination,
RouterRouteCreate,
RouterRouteUpdate,
RouteTarget,
import {
usePrefetchedApiQuery,
type RouteDestination,
type RouterRouteCreate,
type RouterRouteUpdate,
type RouteTarget,
} from '~/api'
import { ComboboxField } from '~/components/form/fields/ComboboxField'
import { DescriptionField } from '~/components/form/fields/DescriptionField'
import { ListboxField } from '~/components/form/fields/ListboxField'
import { NameField } from '~/components/form/fields/NameField'
import { TextField } from '~/components/form/fields/TextField'
import { useVpcRouterSelector } from '~/hooks/use-params'
import { Message } from '~/ui/lib/Message'

export type RouteFormValues = RouterRouteCreate | Required<RouterRouteUpdate>
Expand Down Expand Up @@ -60,7 +63,18 @@ type RouteFormFieldsProps = {
isDisabled?: boolean
}
export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
const routerSelector = useVpcRouterSelector()
const { project, vpc } = routerSelector
// usePrefetchedApiQuery items below are initially fetched in the loaders in vpc-router-route-create and -edit
const {
data: { items: vpcSubnets },
} = usePrefetchedApiQuery('vpcSubnetList', { query: { project, vpc, limit: 1000 } })
const {
data: { items: instances },
} = usePrefetchedApiQuery('instanceList', { query: { project, limit: 1000 } })

const { control } = form
const destinationType = form.watch('destination.type')
const targetType = form.watch('target.type')
return (
<>
Expand All @@ -76,16 +90,31 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
items={toItems(destTypes)}
placeholder="Select a destination type"
required
onChange={() => {
form.setValue('destination.value', '')
}}
disabled={isDisabled}
/>
<TextField
name="destination.value"
label="Destination value"
control={control}
placeholder="Enter a destination value"
required
disabled={isDisabled}
/>
{destinationType === 'subnet' ? (
<ComboboxField
name="destination.value"
label="Destination value"
control={control}
items={vpcSubnets.map(({ name }) => ({ value: name, label: name }))}
placeholder="Select a subnet"
required
isDisabled={isDisabled}
/>
) : (
<TextField
name="destination.value"
label="Destination value"
control={control}
placeholder="Enter a destination value"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do like matching the API shape roughly, but maybe we can tweak some words to make this a little more human-feeling. Maybe it's enough to just change the placeholder based on the type selected, or maybe we want to go as far as changing the label? It would also be nice to get some kind of guidance about the right format for an IP network. I assume it's some kind of block, maybe we can give an example.

image

required
disabled={isDisabled}
/>
)}
<ListboxField
name="target.type"
label="Target type"
Expand All @@ -98,7 +127,17 @@ export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
}}
disabled={isDisabled}
/>
{targetType !== 'drop' && (
{targetType === 'drop' ? null : targetType === 'instance' ? (
<ComboboxField
name="target.value"
label="Target value"
control={control}
items={instances.map(({ name }) => ({ value: name, label: name }))}
placeholder="Select an instance"
required
isDisabled={isDisabled}
/>
) : (
<TextField
name="target.value"
label="Target value"
Expand Down
19 changes: 16 additions & 3 deletions app/forms/vpc-router-route-create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
* Copyright Oxide Computer Company
*/
import { useForm } from 'react-hook-form'
import { useNavigate } from 'react-router-dom'
import { useNavigate, type LoaderFunctionArgs } from 'react-router-dom'

import { useApiMutation, useApiQueryClient } from '@oxide/api'
import { apiQueryClient, useApiMutation, useApiQueryClient } from '@oxide/api'

import { SideModalForm } from '~/components/form/SideModalForm'
import { RouteFormFields, type RouteFormValues } from '~/forms/vpc-router-route-common'
import { useVpcRouterSelector } from '~/hooks/use-params'
import { getVpcRouterSelector, useVpcRouterSelector } from '~/hooks/use-params'
import { addToast } from '~/stores/toast'
import { pb } from '~/util/path-builder'

Expand All @@ -23,6 +23,19 @@ const defaultValues: RouteFormValues = {
target: { type: 'ip', value: '' },
}

CreateRouterRouteSideModalForm.loader = async ({ params }: LoaderFunctionArgs) => {
const { project, vpc } = getVpcRouterSelector(params)
await Promise.all([
apiQueryClient.prefetchQuery('vpcSubnetList', {
query: { project, vpc, limit: 1000 },
charliepark marked this conversation as resolved.
Show resolved Hide resolved
}),
apiQueryClient.prefetchQuery('instanceList', {
query: { project, limit: 1000 },
}),
])
return null
}

export function CreateRouterRouteSideModalForm() {
const queryClient = useApiQueryClient()
const routerSelector = useVpcRouterSelector()
Expand Down
18 changes: 13 additions & 5 deletions app/forms/vpc-router-route-edit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,19 @@ import { addToast } from '~/stores/toast'
import { pb } from '~/util/path-builder'

EditRouterRouteSideModalForm.loader = async ({ params }: LoaderFunctionArgs) => {
const { route, ...routerSelector } = getVpcRouterRouteSelector(params)
await apiQueryClient.prefetchQuery('vpcRouterRouteView', {
path: { route },
query: routerSelector,
})
const { project, vpc, router, route } = getVpcRouterRouteSelector(params)
await Promise.all([
apiQueryClient.prefetchQuery('vpcRouterRouteView', {
path: { route },
query: { project, vpc, router },
}),
apiQueryClient.prefetchQuery('vpcSubnetList', {
query: { project, vpc, limit: 1000 },
}),
apiQueryClient.prefetchQuery('instanceList', {
query: { project, limit: 1000 },
}),
])
return null
}

Expand Down
1 change: 1 addition & 0 deletions app/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ export const routes = createRoutesFromElements(
<Route
path="routes-new"
element={<CreateRouterRouteSideModalForm />}
loader={CreateRouterRouteSideModalForm.loader}
handle={{ crumb: 'New Route' }}
/>
<Route
Expand Down
11 changes: 7 additions & 4 deletions test/e2e/vpcs.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/
import { expect, test } from '@playwright/test'

import { clickRowAction, expectRowVisible } from './utils'
import { clickRowAction, expectRowVisible, selectOption } from './utils'

test('can nav to VpcPage from /', async ({ page }) => {
await page.goto('/')
Expand Down Expand Up @@ -248,13 +248,16 @@ test('can create, update, and delete Route', async ({ page }) => {

// update the route by clicking the edit button
await clickRowAction(page, 'new-route', 'Edit')
await page.getByRole('textbox', { name: 'Destination value' }).fill('0.0.0.1')
// change the destination type to VPC subnet: `mock-subnet`
await selectOption(page, 'Destination type', 'Subnet')
await selectOption(page, 'Destination value', 'mock-subnet')
charliepark marked this conversation as resolved.
Show resolved Hide resolved
await page.getByRole('textbox', { name: 'Target value' }).fill('0.0.0.1')
await page.getByRole('button', { name: 'Update route' }).click()
await expect(routeRows).toHaveCount(2)
await expectRowVisible(table, {
Name: 'new-route',
Destination: 'IP0.0.0.1',
Target: 'IP1.1.1.1',
Destination: 'VPC subnetmock-subnet',
Target: 'IP0.0.0.1',
})

// delete the route
Expand Down
Loading