From e04f4895bf7ce264ff9791f46353be4ebd0a3297 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 5 Mar 2024 10:21:13 -0800 Subject: [PATCH 1/5] Add form to update Floating IP to console ui --- OMICRON_VERSION | 2 +- app/api/__generated__/Api.ts | 36 +++++++++ app/api/__generated__/OMICRON_VERSION | 2 +- app/api/__generated__/msw-handlers.ts | 16 ++++ app/api/__generated__/validate.ts | 20 +++++ app/forms/floating-ip-edit.tsx | 77 +++++++++++++++++++ app/main.tsx | 4 +- .../project/floating-ips/FloatingIpsPage.tsx | 11 ++- app/routes.tsx | 9 ++- app/util/path-builder.spec.ts | 2 + app/util/path-builder.ts | 3 + mock-api/msw/handlers.ts | 18 ++++- 12 files changed, 191 insertions(+), 9 deletions(-) create mode 100644 app/forms/floating-ip-edit.tsx diff --git a/OMICRON_VERSION b/OMICRON_VERSION index ea552e717d..1252df055d 100644 --- a/OMICRON_VERSION +++ b/OMICRON_VERSION @@ -1 +1 @@ -d514878f417a94247791bd5564fbaafa9b4170a0 +1f26c66921b9215bfe11d750514939bcdc11ae12 diff --git a/app/api/__generated__/Api.ts b/app/api/__generated__/Api.ts index 168960d331..4e23de5b34 100644 --- a/app/api/__generated__/Api.ts +++ b/app/api/__generated__/Api.ts @@ -1118,6 +1118,11 @@ export type FloatingIpResultsPage = { nextPage?: string } +/** + * Updateable identity-related parameters + */ +export type FloatingIpUpdate = { description?: string; name?: Name } + /** * View of a Group */ @@ -3116,6 +3121,14 @@ export interface FloatingIpViewQueryParams { project?: NameOrId } +export interface FloatingIpUpdatePathParams { + floatingIp: NameOrId +} + +export interface FloatingIpUpdateQueryParams { + project?: NameOrId +} + export interface FloatingIpDeletePathParams { floatingIp: NameOrId } @@ -4296,6 +4309,29 @@ export class Api extends HttpClient { ...params, }) }, + /** + * Update floating IP + */ + floatingIpUpdate: ( + { + path, + query = {}, + body, + }: { + path: FloatingIpUpdatePathParams + query?: FloatingIpUpdateQueryParams + body: FloatingIpUpdate + }, + params: FetchParams = {} + ) => { + return this.request({ + path: `/v1/floating-ips/${path.floatingIp}`, + method: 'PUT', + body, + query, + ...params, + }) + }, /** * Delete floating IP */ diff --git a/app/api/__generated__/OMICRON_VERSION b/app/api/__generated__/OMICRON_VERSION index e02fa21be7..977aa82d85 100644 --- a/app/api/__generated__/OMICRON_VERSION +++ b/app/api/__generated__/OMICRON_VERSION @@ -1,2 +1,2 @@ # generated file. do not update manually. see docs/update-pinned-api.md -d514878f417a94247791bd5564fbaafa9b4170a0 +1f26c66921b9215bfe11d750514939bcdc11ae12 diff --git a/app/api/__generated__/msw-handlers.ts b/app/api/__generated__/msw-handlers.ts index 43b7a65b90..4c42a38f2f 100644 --- a/app/api/__generated__/msw-handlers.ts +++ b/app/api/__generated__/msw-handlers.ts @@ -175,6 +175,14 @@ export interface MSWHandlers { req: Request cookies: Record }) => Promisable> + /** `PUT /v1/floating-ips/:floatingIp` */ + floatingIpUpdate: (params: { + path: Api.FloatingIpUpdatePathParams + query: Api.FloatingIpUpdateQueryParams + body: Json + req: Request + cookies: Record + }) => Promisable> /** `DELETE /v1/floating-ips/:floatingIp` */ floatingIpDelete: (params: { path: Api.FloatingIpDeletePathParams @@ -1349,6 +1357,14 @@ export function makeHandlers(handlers: MSWHandlers): HttpHandler[] { '/v1/floating-ips/:floatingIp', handler(handlers['floatingIpView'], schema.FloatingIpViewParams, null) ), + http.put( + '/v1/floating-ips/:floatingIp', + handler( + handlers['floatingIpUpdate'], + schema.FloatingIpUpdateParams, + schema.FloatingIpUpdate + ) + ), http.delete( '/v1/floating-ips/:floatingIp', handler(handlers['floatingIpDelete'], schema.FloatingIpDeleteParams, null) diff --git a/app/api/__generated__/validate.ts b/app/api/__generated__/validate.ts index 26d786c244..312837c21f 100644 --- a/app/api/__generated__/validate.ts +++ b/app/api/__generated__/validate.ts @@ -1220,6 +1220,14 @@ export const FloatingIpResultsPage = z.preprocess( z.object({ items: FloatingIp.array(), nextPage: z.string().optional() }) ) +/** + * Updateable identity-related parameters + */ +export const FloatingIpUpdate = z.preprocess( + processResponseBody, + z.object({ description: z.string().optional(), name: Name.optional() }) +) + /** * View of a Group */ @@ -3193,6 +3201,18 @@ export const FloatingIpViewParams = z.preprocess( }) ) +export const FloatingIpUpdateParams = z.preprocess( + processResponseBody, + z.object({ + path: z.object({ + floatingIp: NameOrId, + }), + query: z.object({ + project: NameOrId.optional(), + }), + }) +) + export const FloatingIpDeleteParams = z.preprocess( processResponseBody, z.object({ diff --git a/app/forms/floating-ip-edit.tsx b/app/forms/floating-ip-edit.tsx new file mode 100644 index 0000000000..cab8e03019 --- /dev/null +++ b/app/forms/floating-ip-edit.tsx @@ -0,0 +1,77 @@ +/* + * 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 { useNavigate, type LoaderFunctionArgs } from 'react-router-dom' + +import { + apiQueryClient, + useApiMutation, + useApiQueryClient, + usePrefetchedApiQuery, +} from '@oxide/api' + +import { DescriptionField } from '~/components/form/fields/DescriptionField' +import { NameField } from '~/components/form/fields/NameField' +import { SideModalForm } from '~/components/form/SideModalForm' +import { getFloatingIpSelector, useFloatingIpSelector, useForm, useToast } from 'app/hooks' +import { pb } from 'app/util/path-builder' + +EditFloatingIpSideModalForm.loader = async ({ params }: LoaderFunctionArgs) => { + const { floatingIp, project } = getFloatingIpSelector(params) + await apiQueryClient.prefetchQuery('floatingIpView', { + path: { floatingIp }, + query: { project }, + }) + return null +} + +export function EditFloatingIpSideModalForm() { + const queryClient = useApiQueryClient() + const addToast = useToast() + const navigate = useNavigate() + + const floatingIpSelector = useFloatingIpSelector() + + const onDismiss = () => navigate(pb.floatingIps({ project: floatingIpSelector.project })) + + const { data: floatingIp } = usePrefetchedApiQuery('floatingIpView', { + path: { floatingIp: floatingIpSelector.floatingIp }, + query: { project: floatingIpSelector.project }, + }) + + const editFloatingIp = useApiMutation('floatingIpUpdate', { + onSuccess(_floatingIp) { + queryClient.invalidateQueries('floatingIpList') + addToast({ content: 'Your floating IP has been updated' }) + onDismiss() + }, + }) + + const form = useForm({ defaultValues: floatingIp }) + + return ( + { + editFloatingIp.mutate({ + path: { floatingIp: floatingIpSelector.floatingIp }, + query: { project: floatingIpSelector.project }, + body: { name, description }, + }) + }} + loading={editFloatingIp.isPending} + submitError={editFloatingIp.error} + submitLabel="Save changes" + > + + + + ) +} diff --git a/app/main.tsx b/app/main.tsx index 07583dd030..27d360cfa8 100644 --- a/app/main.tsx +++ b/app/main.tsx @@ -6,7 +6,7 @@ * Copyright Oxide Computer Company */ import { QueryClientProvider } from '@tanstack/react-query' -// import { ReactQueryDevtools } from '@tanstack/react-query-devtools' +import { ReactQueryDevtools } from '@tanstack/react-query-devtools' import { StrictMode } from 'react' import { createRoot } from 'react-dom/client' import { createBrowserRouter, RouterProvider } from 'react-router-dom' @@ -53,7 +53,7 @@ function render() { - {/* */} + ) diff --git a/app/pages/project/floating-ips/FloatingIpsPage.tsx b/app/pages/project/floating-ips/FloatingIpsPage.tsx index cb33ebcda1..44fffd0a82 100644 --- a/app/pages/project/floating-ips/FloatingIpsPage.tsx +++ b/app/pages/project/floating-ips/FloatingIpsPage.tsx @@ -7,7 +7,7 @@ */ import { useState } from 'react' import { useForm } from 'react-hook-form' -import { Link, Outlet, type LoaderFunctionArgs } from 'react-router-dom' +import { Link, Outlet, useNavigate, type LoaderFunctionArgs } from 'react-router-dom' import { apiQueryClient, @@ -66,6 +66,7 @@ export function FloatingIpsPage() { const { data: instances } = usePrefetchedApiQuery('instanceList', { query: { project }, }) + const navigate = useNavigate() const getInstanceName = (instanceId: string) => instances.items.find((i) => i.id === instanceId)?.name @@ -122,6 +123,14 @@ export function FloatingIpsPage() { }, } return [ + { + label: 'Edit', + onActivate: () => { + navigate(pb.floatingIpEdit({ project, floatingIp: floatingIp.name }), { + state: floatingIp, + }) + }, + }, attachOrDetachAction, { label: 'Delete', diff --git a/app/routes.tsx b/app/routes.tsx index f650729c6e..09fc221d80 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -11,6 +11,7 @@ import { RouterDataErrorBoundary } from './components/ErrorBoundary' import { NotFound } from './components/ErrorPage' import { CreateDiskSideModalForm } from './forms/disk-create' import { CreateFloatingIpSideModalForm } from './forms/floating-ip-create' +import { EditFloatingIpSideModalForm } from './forms/floating-ip-edit' import { CreateIdpSideModalForm } from './forms/idp/create' import { EditIdpSideModalForm } from './forms/idp/edit' import { @@ -350,13 +351,19 @@ export const routes = createRoutesFromElements( /> - }> + } loader={FloatingIpsPage.loader}> } handle={{ crumb: 'New Floating IP' }} /> + } + loader={EditFloatingIpSideModalForm.loader} + handle={{ crumb: 'Edit Floating IP' }} + /> } loader={DisksPage.loader}> diff --git a/app/util/path-builder.spec.ts b/app/util/path-builder.spec.ts index 1748e482ad..13190a1d79 100644 --- a/app/util/path-builder.spec.ts +++ b/app/util/path-builder.spec.ts @@ -11,6 +11,7 @@ import { pb } from './path-builder' // params can be the same for all of them because they only use what they need const params = { + floatingIp: 'f', project: 'p', instance: 'i', vpc: 'v', @@ -31,6 +32,7 @@ test('path builder', () => { "diskInventory": "/system/inventory/disks", "disks": "/projects/p/disks", "disksNew": "/projects/p/disks-new", + "floatingIpEdit": "/projects/p/floating-ips/f/edit", "floatingIps": "/projects/p/floating-ips", "floatingIpsNew": "/projects/p/floating-ips-new", "instance": "/projects/p/instances/i", diff --git a/app/util/path-builder.ts b/app/util/path-builder.ts index 817a04df7a..9aa83b2116 100644 --- a/app/util/path-builder.ts +++ b/app/util/path-builder.ts @@ -20,6 +20,7 @@ type Image = Required type Snapshot = Required type SiloImage = Required type IpPool = Required +type FloatingIp = Required export const pb = { projects: () => `/projects`, @@ -68,6 +69,8 @@ export const pb = { vpcEdit: (params: Vpc) => `${pb.vpc(params)}/edit`, floatingIps: (params: Project) => `${pb.project(params)}/floating-ips`, floatingIpsNew: (params: Project) => `${pb.project(params)}/floating-ips-new`, + floatingIp: (params: FloatingIp) => `${pb.floatingIps(params)}/${params.floatingIp}`, + floatingIpEdit: (params: FloatingIp) => `${pb.floatingIp(params)}/edit`, siloUtilization: () => '/utilization', siloAccess: () => '/access', diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 0f56dcaf95..ebfe39c207 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -246,6 +246,20 @@ export const handlers = makeHandlers({ return paginated(query, ips) }, floatingIpView: ({ path, query }) => lookup.floatingIp({ ...path, ...query }), + floatingIpUpdate: ({ path, query, body }) => { + const floatingIp = lookup.floatingIp({ ...path, ...query }) + if (body.name) { + // only check for existing name if it's being changed + if (body.name !== floatingIp.name) { + errIfExists(db.floatingIps, { name: body.name }) + } + floatingIp.name = body.name + } + if (body.description) { + floatingIp.description = body.description + } + return floatingIp + }, floatingIpDelete({ path, query }) { const floatingIp = lookup.floatingIp({ ...path, ...query }) db.floatingIps = db.floatingIps.filter((i) => i.id !== floatingIp.id) @@ -554,9 +568,7 @@ export const handlers = makeHandlers({ if (body.name) { nic.name = body.name } - if (typeof body.description === 'string') { - nic.description = body.description - } + nic.description = body.description || '' if (typeof body.primary === 'boolean' && body.primary !== nic.primary) { if (nic.primary) { From 76286b326e84cb5045992080a36c90bf5b600a3f Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 5 Mar 2024 11:23:23 -0800 Subject: [PATCH 2/5] Add e2e test for updating a floating IP --- test/e2e/floating-ip-update.e2e.ts | 61 ++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 test/e2e/floating-ip-update.e2e.ts diff --git a/test/e2e/floating-ip-update.e2e.ts b/test/e2e/floating-ip-update.e2e.ts new file mode 100644 index 0000000000..f32a976991 --- /dev/null +++ b/test/e2e/floating-ip-update.e2e.ts @@ -0,0 +1,61 @@ +/* + * 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 { clickRowAction, expect, expectRowVisible, expectVisible, test } from './utils' + +const floatingIpsPage = '/projects/mock-project/floating-ips' +const originalName = 'cola-float' +const updatedName = 'updated-cola-float' +const updatedDescription = 'An updated description for this Floating IP' + +test('can update a Floating IP', async ({ page }) => { + await page.goto(floatingIpsPage) + await clickRowAction(page, 'cola-float', 'Edit') + + await expectVisible(page, [ + 'role=heading[name*="Edit floating IP"]', + 'role=textbox[name="Name"]', + 'role=textbox[name="Description"]', + 'role=button[name="Save changes"]', + ]) + + await page.fill('input[name=name]', updatedName) + await page.getByRole('textbox', { name: 'Description' }).fill(updatedDescription) + + await page.getByRole('button', { name: 'Save changes' }).click() + + await expect(page).toHaveURL(floatingIpsPage) + + await expectRowVisible(page.getByRole('table'), { + name: updatedName, + description: updatedDescription, + }) +}) + +test('updating a Floating IP description works (even without updating name)', async ({ + page, +}) => { + await page.goto(floatingIpsPage) + await clickRowAction(page, originalName, 'Edit') + + await expectVisible(page, [ + 'role=heading[name*="Edit floating IP"]', + 'role=textbox[name="Name"]', + 'role=textbox[name="Description"]', + 'role=button[name="Save changes"]', + ]) + + const updatedDescription = 'An updated description for this Floating IP' + await page.getByRole('textbox', { name: 'Description' }).fill(updatedDescription) + await page.getByRole('button', { name: 'Save changes' }).click() + await expect(page).toHaveURL(floatingIpsPage) + await expectRowVisible(page.getByRole('table'), { + name: originalName, + description: updatedDescription, + }) +}) From 21e2989ed02e6348f8c637c85eba80ae87af9317 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 5 Mar 2024 11:25:28 -0800 Subject: [PATCH 3/5] Re-hide ReactQueryDevTools --- app/main.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/main.tsx b/app/main.tsx index 27d360cfa8..07583dd030 100644 --- a/app/main.tsx +++ b/app/main.tsx @@ -6,7 +6,7 @@ * Copyright Oxide Computer Company */ import { QueryClientProvider } from '@tanstack/react-query' -import { ReactQueryDevtools } from '@tanstack/react-query-devtools' +// import { ReactQueryDevtools } from '@tanstack/react-query-devtools' import { StrictMode } from 'react' import { createRoot } from 'react-dom/client' import { createBrowserRouter, RouterProvider } from 'react-router-dom' @@ -53,7 +53,7 @@ function render() { - + {/* */} ) From ab22747c13daa72fb4597a5f1c43236c3898b4ec Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 5 Mar 2024 11:41:06 -0800 Subject: [PATCH 4/5] Updated path-builder spec with npm test run -- -u --- app/util/path-builder.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/app/util/path-builder.spec.ts b/app/util/path-builder.spec.ts index 13190a1d79..4e3da60ede 100644 --- a/app/util/path-builder.spec.ts +++ b/app/util/path-builder.spec.ts @@ -32,6 +32,7 @@ test('path builder', () => { "diskInventory": "/system/inventory/disks", "disks": "/projects/p/disks", "disksNew": "/projects/p/disks-new", + "floatingIp": "/projects/p/floating-ips/f", "floatingIpEdit": "/projects/p/floating-ips/f/edit", "floatingIps": "/projects/p/floating-ips", "floatingIpsNew": "/projects/p/floating-ips-new", From 4c7ca63a29dab9906054c7800b7cd7ee1f721cee Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 5 Mar 2024 12:44:52 -0800 Subject: [PATCH 5/5] Refactor, ensure uniqueness, preload data for modal --- .../project/floating-ips/FloatingIpsPage.tsx | 12 ++++-- mock-api/msw/handlers.ts | 2 +- test/e2e/floating-ip-update.e2e.ts | 39 +++++++------------ 3 files changed, 24 insertions(+), 29 deletions(-) diff --git a/app/pages/project/floating-ips/FloatingIpsPage.tsx b/app/pages/project/floating-ips/FloatingIpsPage.tsx index 44fffd0a82..409ef30660 100644 --- a/app/pages/project/floating-ips/FloatingIpsPage.tsx +++ b/app/pages/project/floating-ips/FloatingIpsPage.tsx @@ -126,9 +126,15 @@ export function FloatingIpsPage() { { label: 'Edit', onActivate: () => { - navigate(pb.floatingIpEdit({ project, floatingIp: floatingIp.name }), { - state: floatingIp, - }) + apiQueryClient.setQueryData( + 'floatingIpView', + { + path: { floatingIp: floatingIp.name }, + query: { project }, + }, + floatingIp + ) + navigate(pb.floatingIpEdit({ project, floatingIp: floatingIp.name })) }, }, attachOrDetachAction, diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index ebfe39c207..8104244073 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -251,7 +251,7 @@ export const handlers = makeHandlers({ if (body.name) { // only check for existing name if it's being changed if (body.name !== floatingIp.name) { - errIfExists(db.floatingIps, { name: body.name }) + errIfExists(db.floatingIps, { name: body.name, project_id: floatingIp.project_id }) } floatingIp.name = body.name } diff --git a/test/e2e/floating-ip-update.e2e.ts b/test/e2e/floating-ip-update.e2e.ts index f32a976991..0453358eeb 100644 --- a/test/e2e/floating-ip-update.e2e.ts +++ b/test/e2e/floating-ip-update.e2e.ts @@ -12,45 +12,34 @@ const floatingIpsPage = '/projects/mock-project/floating-ips' const originalName = 'cola-float' const updatedName = 'updated-cola-float' const updatedDescription = 'An updated description for this Floating IP' - -test('can update a Floating IP', async ({ page }) => { +const expectedFormElements = [ + 'role=heading[name*="Edit floating IP"]', + 'role=textbox[name="Name"]', + 'role=textbox[name="Description"]', + 'role=button[name="Save changes"]', +] + +test('can update a floating IP', async ({ page }) => { await page.goto(floatingIpsPage) await clickRowAction(page, 'cola-float', 'Edit') - - await expectVisible(page, [ - 'role=heading[name*="Edit floating IP"]', - 'role=textbox[name="Name"]', - 'role=textbox[name="Description"]', - 'role=button[name="Save changes"]', - ]) + await expectVisible(page, expectedFormElements) await page.fill('input[name=name]', updatedName) await page.getByRole('textbox', { name: 'Description' }).fill(updatedDescription) - await page.getByRole('button', { name: 'Save changes' }).click() - await expect(page).toHaveURL(floatingIpsPage) - await expectRowVisible(page.getByRole('table'), { name: updatedName, description: updatedDescription, }) }) -test('updating a Floating IP description works (even without updating name)', async ({ - page, -}) => { - await page.goto(floatingIpsPage) - await clickRowAction(page, originalName, 'Edit') - - await expectVisible(page, [ - 'role=heading[name*="Edit floating IP"]', - 'role=textbox[name="Name"]', - 'role=textbox[name="Description"]', - 'role=button[name="Save changes"]', - ]) +// Make sure that it still works even if the name doesn't change +test('can update *just* the floating IP description', async ({ page }) => { + // Go to the edit page for the original floating IP + await page.goto(`${floatingIpsPage}/${originalName}/edit`) + await expectVisible(page, expectedFormElements) - const updatedDescription = 'An updated description for this Floating IP' await page.getByRole('textbox', { name: 'Description' }).fill(updatedDescription) await page.getByRole('button', { name: 'Save changes' }).click() await expect(page).toHaveURL(floatingIpsPage)