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

* Use human-friendly copy on placeholders and descriptions

Author: charliepark

app/api/hooks.ts

@@ -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
 • 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

app/forms/vpc-router-route-common.tsx

@@ -8,16 +8,21 @@
 
 import type { UseFormReturn } from 'react-hook-form'
 
-import type {
-  RouteDestination,
-  RouterRouteCreate,
-  RouterRouteUpdate,
-  RouteTarget,
+import {
+  usePrefetchedApiQuery,
+  type Instance,
+  type RouteDestination,
+  type RouterRouteCreate,
+  type RouterRouteUpdate,
+  type RouteTarget,
+  type VpcSubnet,
 } 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>
@@ -52,65 +57,122 @@ const targetTypes: Record<Exclude<RouteTarget['type'], 'subnet' | 'vpc'>, string
   drop: 'Drop',
 }
 
-const toItems = (mapping: Record<string, string>) =>
+const destinationValuePlaceholder: Record<RouteDestination['type'], string | undefined> = {
+  ip: 'Enter an IP',
+  ip_net: 'Enter an IP network',
+  subnet: 'Select a subnet',
+  vpc: undefined,
+}
+
+const destinationValueDescription: Record<RouteDestination['type'], string | undefined> = {
+  ip: 'An IP address, like 192.168.1.222',
+  ip_net: 'An IP network, like 192.168.0.0/16',
+  subnet: undefined,
+  vpc: undefined,
+}
+
+/** possible targetTypes needing placeholders are instances or IPs (internet_gateway has no placeholder) */
+const targetValuePlaceholder: Record<RouteTarget['type'], string | undefined> = {
+  ip: 'Enter an IP',
+  instance: 'Select an instance',
+  internet_gateway: undefined,
+  drop: undefined,
+  subnet: undefined,
+  vpc: undefined,
+}
+
+const targetValueDescription: Record<RouteTarget['type'], string | undefined> = {
+  ip: 'An IP address, like 10.0.1.5',
+  instance: undefined,
+  internet_gateway: routeFormMessage.internetGatewayTargetValue,
+  drop: undefined,
+  subnet: undefined,
+  vpc: undefined,
+}
+
+const toListboxItems = (mapping: Record<string, string>) =>
   Object.entries(mapping).map(([value, label]) => ({ value, label }))
 
+const toComboboxItems = (items: Array<Instance | VpcSubnet>) =>
+  items.map(({ name }) => ({ value: name, label: name }))
+
 type RouteFormFieldsProps = {
   form: UseFormReturn<RouteFormValues>
-  isDisabled?: boolean
+  disabled?: boolean
 }
-export const RouteFormFields = ({ form, isDisabled }: RouteFormFieldsProps) => {
+export const RouteFormFields = ({ form, disabled }: 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')
+  const destinationValueProps = {
+    name: 'destination.value' as const,
+    label: 'Destination value',
+    control,
+    placeholder: destinationValuePlaceholder[destinationType],
+    required: true,
+    disabled,
+    description: destinationValueDescription[destinationType],
+  }
+  const targetValueProps = {
+    name: 'target.value' as const,
+    label: 'Target value',
+    control,
+    placeholder: targetValuePlaceholder[targetType],
+    required: true,
+    // 'internet_gateway' targetTypes can only have the value 'outbound', so we disable the field
+    disabled: disabled || targetType === 'internet_gateway',
+    description: targetValueDescription[targetType],
+  }
   return (
     <>
-      {isDisabled && (
+      {disabled && (
         <Message variant="info" content={routeFormMessage.vpcSubnetNotModifiable} />
       )}
-      <NameField name="name" control={control} disabled={isDisabled} />
-      <DescriptionField name="description" control={control} disabled={isDisabled} />
+      <NameField name="name" control={control} disabled={disabled} />
+      <DescriptionField name="description" control={control} disabled={disabled} />
       <ListboxField
         name="destination.type"
         label="Destination type"
         control={control}
-        items={toItems(destTypes)}
+        items={toListboxItems(destTypes)}
         placeholder="Select a destination type"
         required
-        disabled={isDisabled}
-      />
-      <TextField
-        name="destination.value"
-        label="Destination value"
-        control={control}
-        placeholder="Enter a destination value"
-        required
-        disabled={isDisabled}
+        onChange={() => {
+          form.setValue('destination.value', '')
+        }}
+        disabled={disabled}
       />
+      {destinationType === 'subnet' ? (
+        <ComboboxField {...destinationValueProps} items={toComboboxItems(vpcSubnets)} />
+      ) : (
+        <TextField {...destinationValueProps} />
+      )}
       <ListboxField
         name="target.type"
         label="Target type"
         control={control}
-        items={toItems(targetTypes)}
+        items={toListboxItems(targetTypes)}
         placeholder="Select a target type"
         required
         onChange={(value) => {
           form.setValue('target.value', value === 'internet_gateway' ? 'outbound' : '')
         }}
-        disabled={isDisabled}
+        disabled={disabled}
       />
-      {targetType !== 'drop' && (
-        <TextField
-          name="target.value"
-          label="Target value"
-          control={control}
-          placeholder="Enter a target value"
-          required
-          // 'internet_gateway' targetTypes can only have the value 'outbound', so we disable the field
-          disabled={isDisabled || targetType === 'internet_gateway'}
-          description={
-            targetType === 'internet_gateway' && routeFormMessage.internetGatewayTargetValue
-          }
-        />
+      {targetType === 'drop' ? null : targetType === 'instance' ? (
+        <ComboboxField {...targetValueProps} items={toComboboxItems(instances)} />
+      ) : (
+        <TextField {...targetValueProps} />
       )}
     </>
   )

app/forms/vpc-router-route-create.tsx

@@ -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'
 
@@ -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 },
+    }),
+    apiQueryClient.prefetchQuery('instanceList', {
+      query: { project, limit: 1000 },
+    }),
+  ])
+  return null
+}
+
 export function CreateRouterRouteSideModalForm() {
   const queryClient = useApiQueryClient()
   const routerSelector = useVpcRouterSelector()

app/forms/vpc-router-route-edit.tsx

@@ -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
 }
 
@@ -51,7 +59,7 @@ export function EditRouterRouteSideModalForm() {
     'destination',
   ])
   const form = useForm({ defaultValues })
-  const isDisabled = route?.kind === 'vpc_subnet'
+  const disabled = route?.kind === 'vpc_subnet'
 
   const updateRouterRoute = useApiMutation('vpcRouterRouteUpdate', {
     onSuccess() {
@@ -82,9 +90,9 @@ export function EditRouterRouteSideModalForm() {
       }
       loading={updateRouterRoute.isPending}
       submitError={updateRouterRoute.error}
-      submitDisabled={isDisabled ? routeFormMessage.vpcSubnetNotModifiable : undefined}
+      submitDisabled={disabled ? routeFormMessage.vpcSubnetNotModifiable : undefined}
     >
-      <RouteFormFields form={form} isDisabled={isDisabled} />
+      <RouteFormFields form={form} disabled={disabled} />
     </SideModalForm>
   )
 }

app/routes.tsx

@@ -424,6 +424,7 @@ export const routes = createRoutesFromElements(
           <Route
             path="routes-new"
             element={<CreateRouterRouteSideModalForm />}
+            loader={CreateRouterRouteSideModalForm.loader}
             handle={{ crumb: 'New Route' }}
           />
           <Route

test/e2e/vpcs.e2e.ts

@@ -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('/')
@@ -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')
+  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