From 8028f9a5e05eb855a4e39457a12bdfc2d9e15900 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Thu, 22 Aug 2024 16:32:24 -0700 Subject: [PATCH] Choose custom router on subnet create/edit forms (#2393) * Add combobox to subnet create and edit forms --------- Co-authored-by: David Crespo --- app/components/form/fields/useItemsList.ts | 44 +++++++++ app/forms/subnet-create.tsx | 36 ++++++- app/forms/subnet-edit.tsx | 31 +++++- .../vpcs/VpcPage/tabs/VpcSubnetsTab.tsx | 7 ++ app/table/cells/RouterLinkCell.tsx | 36 +++++++ mock-api/msw/handlers.ts | 10 +- mock-api/vpc.ts | 1 + test/e2e/vpcs.e2e.ts | 99 +++++++++++++++---- 8 files changed, 239 insertions(+), 25 deletions(-) create mode 100644 app/components/form/fields/useItemsList.ts create mode 100644 app/table/cells/RouterLinkCell.tsx diff --git a/app/components/form/fields/useItemsList.ts b/app/components/form/fields/useItemsList.ts new file mode 100644 index 0000000000..f364899c84 --- /dev/null +++ b/app/components/form/fields/useItemsList.ts @@ -0,0 +1,44 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ + +import { useMemo } from 'react' + +import { useApiQuery } from '~/api' +import { useVpcSelector } from '~/hooks' + +/** + * Special value indicating no router. Must use helper functions to convert + * `undefined` to this when populating form, and this back to `undefined` in + * onSubmit. + */ +const NO_ROUTER = '||no router||' + +/** Convert form value to value for PUT body */ +export function customRouterFormToData(value: string): string | undefined { + return value === NO_ROUTER ? undefined : value +} + +/** Convert value from response body to form value */ +export function customRouterDataToForm(value: string | undefined): string { + return value || NO_ROUTER +} + +export const useCustomRouterItems = () => { + const vpcSelector = useVpcSelector() + const { data, isLoading } = useApiQuery('vpcRouterList', { query: vpcSelector }) + + const routerItems = useMemo(() => { + const items = (data?.items || []) + .filter((item) => item.kind === 'custom') + .map((router) => ({ value: router.id, label: router.name })) + + return [{ value: NO_ROUTER, label: 'None' }, ...items] + }, [data]) + + return { isLoading, items: routerItems } +} diff --git a/app/forms/subnet-create.tsx b/app/forms/subnet-create.tsx index 0ae3de8696..a1c94a1577 100644 --- a/app/forms/subnet-create.tsx +++ b/app/forms/subnet-create.tsx @@ -10,17 +10,27 @@ import { useNavigate } from 'react-router-dom' import { useApiMutation, useApiQueryClient, type VpcSubnetCreate } from '@oxide/api' 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 { + customRouterDataToForm, + customRouterFormToData, + useCustomRouterItems, +} from '~/components/form/fields/useItemsList' import { SideModalForm } from '~/components/form/SideModalForm' import { useForm, useVpcSelector } from '~/hooks' import { FormDivider } from '~/ui/lib/Divider' import { pb } from '~/util/path-builder' -const defaultValues: VpcSubnetCreate = { +const defaultValues: Required = { name: '', description: '', ipv4Block: '', + ipv6Block: '', + // populate the form field with the value corresponding to an undefined custom + // router on a subnet response + customRouter: customRouterDataToForm(undefined), } export function CreateSubnetForm() { @@ -38,6 +48,7 @@ export function CreateSubnetForm() { }) const form = useForm({ defaultValues }) + const { isLoading, items } = useCustomRouterItems() return ( createSubnet.mutate({ query: vpcSelector, body })} + onSubmit={({ name, description, ipv4Block, ipv6Block, customRouter }) => + createSubnet.mutate({ + query: vpcSelector, + body: { + name, + description, + ipv4Block, + ipv6Block: ipv6Block.trim() || undefined, + customRouter: customRouterFormToData(customRouter), + }, + }) + } loading={createSubnet.isPending} submitError={createSubnet.error} > @@ -54,6 +76,16 @@ export function CreateSubnetForm() { + + ) } diff --git a/app/forms/subnet-edit.tsx b/app/forms/subnet-edit.tsx index 5ae5fbd5bd..a8b5863c97 100644 --- a/app/forms/subnet-edit.tsx +++ b/app/forms/subnet-edit.tsx @@ -6,7 +6,6 @@ * Copyright Oxide Computer Company */ import { useNavigate, type LoaderFunctionArgs } from 'react-router-dom' -import * as R from 'remeda' import { apiQueryClient, @@ -17,9 +16,16 @@ import { } from '@oxide/api' import { DescriptionField } from '~/components/form/fields/DescriptionField' +import { ListboxField } from '~/components/form/fields/ListboxField' import { NameField } from '~/components/form/fields/NameField' +import { + customRouterDataToForm, + customRouterFormToData, + useCustomRouterItems, +} from '~/components/form/fields/useItemsList' import { SideModalForm } from '~/components/form/SideModalForm' import { getVpcSubnetSelector, useForm, useVpcSubnetSelector } from '~/hooks' +import { FormDivider } from '~/ui/lib/Divider' import { pb } from '~/util/path-builder' EditSubnetForm.loader = async ({ params }: LoaderFunctionArgs) => { @@ -50,9 +56,14 @@ export function EditSubnetForm() { }, }) - const defaultValues: VpcSubnetUpdate = R.pick(subnet, ['name', 'description']) + const defaultValues: Required = { + name: subnet.name, + description: subnet.description, + customRouter: customRouterDataToForm(subnet.customRouterId), + } const form = useForm({ defaultValues }) + const { isLoading, items } = useCustomRouterItems() return ( + + ) } diff --git a/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx b/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx index 8c0a56dd51..ca66dd0313 100644 --- a/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx +++ b/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx @@ -19,6 +19,7 @@ import { import { getVpcSelector, useVpcSelector } from '~/hooks' import { confirmDelete } from '~/stores/confirm-delete' import { makeLinkCell } from '~/table/cells/LinkCell' +import { RouterLinkCell } from '~/table/cells/RouterLinkCell' import { TwoLineCell } from '~/table/cells/TwoLineCell' import { getActionsCol, type MenuAction } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' @@ -75,10 +76,16 @@ export function VpcSubnetsTab() { colHelper.accessor('name', { cell: makeLinkCell((subnet) => pb.vpcSubnetsEdit({ ...vpcSelector, subnet })), }), + colHelper.accessor('description', Columns.description), colHelper.accessor((vpc) => [vpc.ipv4Block, vpc.ipv6Block] as const, { header: 'IP Block', cell: (info) => , }), + colHelper.accessor('customRouterId', { + header: 'Custom Router', + // RouterLinkCell needed, as we need to convert the customRouterId to the custom router's name + cell: (info) => , + }), colHelper.accessor('timeCreated', Columns.timeCreated), getActionsCol(makeActions), ], diff --git a/app/table/cells/RouterLinkCell.tsx b/app/table/cells/RouterLinkCell.tsx new file mode 100644 index 0000000000..a56b52eab6 --- /dev/null +++ b/app/table/cells/RouterLinkCell.tsx @@ -0,0 +1,36 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ + +import { useApiQuery } from '~/api' +import { useVpcSelector } from '~/hooks' +import { Badge } from '~/ui/lib/Badge' +import { pb } from '~/util/path-builder' + +import { EmptyCell, SkeletonCell } from './EmptyCell' +import { LinkCell } from './LinkCell' + +export const RouterLinkCell = ({ value }: { value?: string }) => { + const { project, vpc } = useVpcSelector() + const { data: router, isError } = useApiQuery( + 'vpcRouterView', + { + path: { router: value! }, + query: { project, vpc }, + }, + { enabled: !!value } + ) + if (!value) return + // probably not possible but let’s be safe + if (isError) return Deleted + if (!router) return // loading + return ( + + {router.name} + + ) +} diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index df21652cf7..29dff60b8c 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -1151,6 +1151,7 @@ export const handlers = makeHandlers({ const subnets = db.vpcSubnets.filter((s) => s.vpc_id === vpc.id) return paginated(query, subnets) }, + vpcSubnetCreate({ body, query }) { const vpc = lookup.vpc(query) errIfExists(db.vpcSubnets, { vpc_id: vpc.id, name: body.name }) @@ -1159,7 +1160,10 @@ export const handlers = makeHandlers({ const newSubnet: Json = { id: uuid(), vpc_id: vpc.id, - ...body, + name: body.name, + description: body.description, + ipv4_block: body.ipv4_block, + custom_router_id: body.custom_router, // required in subnet create but not in update, so we need a fallback. // API says "A random `/64` block will be assigned if one is not // provided." Our fallback is not random, but it should be good enough. @@ -1178,6 +1182,10 @@ export const handlers = makeHandlers({ } updateDesc(subnet, body) + // match the API's arguably undesirable behavior -- key + // not present and value of null are treated the same + subnet.custom_router_id = body.custom_router + return subnet }, vpcSubnetDelete({ path, query }) { diff --git a/mock-api/vpc.ts b/mock-api/vpc.ts index 585e511cf9..b575041b96 100644 --- a/mock-api/vpc.ts +++ b/mock-api/vpc.ts @@ -155,6 +155,7 @@ export const vpcSubnet2: Json = { name: 'mock-subnet-2', vpc_id: vpc.id, ipv4_block: '10.1.1.2/24', + custom_router_id: customRouter.id, } export function defaultFirewallRules(vpcId: string): Json { diff --git a/test/e2e/vpcs.e2e.ts b/test/e2e/vpcs.e2e.ts index 3e1b5f7f63..f9ee5a2078 100644 --- a/test/e2e/vpcs.e2e.ts +++ b/test/e2e/vpcs.e2e.ts @@ -45,34 +45,95 @@ test('can create and delete subnet', async ({ page }) => { await page.goto('/projects/mock-project/vpcs/mock-vpc') await page.getByRole('tab', { name: 'Subnets' }).click() // only one row in table, the default mock-subnet - const rows = page.locator('tbody >> tr') - await expect(rows).toHaveCount(1) - await expect(rows.nth(0).getByText('mock-subnet')).toBeVisible() + const table = page.getByRole('table') + const rows = page.getByRole('table').getByRole('row') + await expect(rows).toHaveCount(2) + + await expectRowVisible(table, { + name: 'mock-subnet', + 'Custom Router': '—', + 'IP Block': expect.stringContaining('10.1.1.1/24'), + }) // open modal, fill out form, submit - await page.click('text=New subnet') - await page.fill('input[name=ipv4Block]', '10.1.1.2/24') - await page.fill('input[name=name]', 'mock-subnet-2') - await page.click('button:has-text("Create subnet")') + await page.getByRole('link', { name: 'New subnet' }).click() - await expect(rows).toHaveCount(2) + const dialog = page.getByRole('dialog', { name: 'Create subnet' }) + await expect(dialog).toBeVisible() - await expect(rows.nth(0).getByText('mock-subnet')).toBeVisible() - await expect(rows.nth(0).getByText('10.1.1.1/24')).toBeVisible() + await dialog.getByRole('textbox', { name: 'Name' }).fill('mock-subnet-2') + await dialog.getByRole('textbox', { name: 'IPv4 block' }).fill('10.1.1.2/24') - await expect(rows.nth(1).getByText('mock-subnet-2')).toBeVisible() - await expect(rows.nth(1).getByText('10.1.1.2/24')).toBeVisible() + // little hack to catch a bug where we weren't handling empty input here properly + await dialog.getByRole('textbox', { name: 'IPv6 block' }).fill('abc') + await dialog.getByRole('textbox', { name: 'IPv6 block' }).clear() - // click more button on row to get menu, then click Delete - await page - .locator('role=row', { hasText: 'mock-subnet-2' }) - .locator('role=button[name="Row actions"]') - .click() + await dialog.getByRole('button', { name: 'Create subnet' }).click() + + await expect(dialog).toBeHidden() + await expect(rows).toHaveCount(3) - await page.getByRole('menuitem', { name: 'Delete' }).click() + await expectRowVisible(table, { + name: 'mock-subnet', + 'Custom Router': '—', + 'IP Block': expect.stringContaining('10.1.1.1/24'), + }) + await expectRowVisible(table, { + name: 'mock-subnet-2', + 'Custom Router': '—', + 'IP Block': expect.stringContaining('10.1.1.2/24'), + }) + + // click more button on row to get menu, then click Delete + await clickRowAction(page, 'mock-subnet-2', 'Delete') await page.getByRole('button', { name: 'Confirm' }).click() - await expect(rows).toHaveCount(1) + await expect(rows).toHaveCount(2) +}) + +test('can create and update subnets with a custom router', async ({ page }) => { + await page.goto('/projects/mock-project/vpcs/mock-vpc/subnets') + await page.getByRole('link', { name: 'New subnet' }).click() + + const table = page.getByRole('table') + const rows = table.getByRole('row') + await expect(rows).toHaveCount(2) + await expectRowVisible(table, { + name: 'mock-subnet', + 'Custom Router': '—', + 'IP Block': expect.stringContaining('10.1.1.1/24'), + }) + + const dialog = page.getByRole('dialog', { name: 'Create subnet' }) + await expect(dialog).toBeVisible() + + await page.getByRole('textbox', { name: 'Name' }).fill('mock-subnet-2') + await page.getByRole('textbox', { name: 'IPv4 block' }).fill('10.1.1.2/24') + + await page.getByRole('button', { name: 'Custom router' }).click() + await page.getByRole('option', { name: 'mock-custom-router' }).click() + + await page.getByRole('button', { name: 'Create subnet' }).click() + await expect(dialog).toBeHidden() + + await expect(rows).toHaveCount(3) + await expectRowVisible(table, { + name: 'mock-subnet-2', + 'Custom Router': 'mock-custom-router', + 'IP Block': expect.stringContaining('10.1.1.2/24'), + }) + + // now remove the router + await page.getByRole('link', { name: 'mock-subnet-2' }).click() + await page.getByRole('button', { name: 'Custom router' }).click() + await page.getByRole('option', { name: 'None' }).click() + await page.getByRole('button', { name: 'Update subnet' }).click() + await expect(dialog).toBeHidden() + + await expectRowVisible(table, { + name: 'mock-subnet-2', + 'Custom Router': '—', + }) }) test('can create, update, and delete Router', async ({ page }) => {