Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions app/components/form/fields/TextField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ export interface TextFieldProps<
units?: string
validate?: Validate<FieldPathValue<TFieldValues, TName>, TFieldValues>
control: Control<TFieldValues>
/** Alters the value of the input during the field's onChange event. */
transform?: (value: string) => FieldPathValue<TFieldValues, TName>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I changed the return type from string | undefined to this, which effectively says it returns whatever the type of the field in question is. The explicit string | undefined was fine for now, but we'd run into a type error later when we tried to use this on a field with a different type.

}

export function TextField<
Expand Down Expand Up @@ -119,6 +121,7 @@ export const TextFieldInner = <
tooltipText,
required,
id: idProp,
transform,
...props
}: TextFieldProps<TFieldValues, TName> & UITextAreaProps) => {
const generatedId = useId()
Expand All @@ -144,6 +147,10 @@ export const TextFieldInner = <
// for the calling code despite the actual input value necessarily
// being a string.
onChange={(e) => {
if (transform) {
onChange(transform(e.target.value))
return
}
if (type === 'number') {
if (e.target.value.trim() === '') {
onChange(0)
Expand Down
7 changes: 6 additions & 1 deletion app/forms/network-interface-create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ export default function CreateNetworkInterfaceForm({
required
control={form.control}
/>
<TextField name="ip" label="IP Address" control={form.control} />
<TextField
name="ip"
label="IP Address"
control={form.control}
transform={(ip) => (ip.trim() === '' ? undefined : ip)}
/>
</SideModalForm>
)
}
78 changes: 78 additions & 0 deletions app/test/e2e/network-interface-create.e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* 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 { test } from '@playwright/test'

import { expect, expectRowVisible } from './utils'

test('can create a NIC with a specified IP address', async ({ page }) => {
// go to an instance’s Network Interfaces page
await page.goto('/projects/mock-project/instances/db1/network-interfaces')

// stop the instance
await page.getByRole('button', { name: 'Instance actions' }).click()
await page.getByRole('menuitem', { name: 'Stop' }).click()

// open the add network interface side modal
await page.getByRole('button', { name: 'Add network interface' }).click()

// fill out the form
await page.getByLabel('Name').fill('nic-1')
await page.getByRole('button', { name: 'VPC' }).click()
await page.getByRole('option', { name: 'mock-vpc' }).click()
await page.getByRole('button', { name: 'Subnet' }).click()
await page.getByRole('option', { name: 'mock-subnet' }).click()
await page.getByLabel('IP Address').fill('1.2.3.4')

const sidebar = page.getByRole('dialog', { name: 'Add network interface' })

// test that the form can be submitted and a new network interface is created
await sidebar.getByRole('button', { name: 'Add network interface' }).click()
await expect(sidebar).toBeHidden()

await expectRowVisible(page.getByRole('table'), { name: 'nic-1', ip: '1.2.3.4' })
})

test('can create a NIC with a blank IP address', async ({ page }) => {
// go to an instance’s Network Interfaces page
await page.goto('/projects/mock-project/instances/db1/network-interfaces')

// stop the instance
await page.getByRole('button', { name: 'Instance actions' }).click()
await page.getByRole('menuitem', { name: 'Stop' }).click()

// open the add network interface side modal
await page.getByRole('button', { name: 'Add network interface' }).click()

// fill out the form
await page.getByLabel('Name').fill('nic-2')
await page.getByRole('button', { name: 'VPC' }).click()
await page.getByRole('option', { name: 'mock-vpc' }).click()
await page.getByRole('button', { name: 'Subnet' }).click()
await page.getByRole('option', { name: 'mock-subnet' }).click()

// make sure the IP address field has a non-conforming bit of text in it
await page.getByLabel('IP Address').fill('x')

// try to submit it
const sidebar = page.getByRole('dialog', { name: 'Add network interface' })
await sidebar.getByRole('button', { name: 'Add network interface' }).click()

// it should error out
// todo: improve error message from API
await expect(sidebar.getByText('Unknown server error')).toBeVisible()
Copy link
Collaborator

@david-crespo david-crespo Dec 13, 2023

Choose a reason for hiding this comment

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

This is because we don't handle MSW errors properly in the UI. One thing we could do, it just occurred to me, is make them look more like real API errors so that the UI would handle them naturally. As in, where right now we just return the Zod error directly as JSON, we could stick it somewhere in an object that looks like { "error_code": "InvalidRequest", "message": "..." }. The Zod error would probably look like garbage stringified, but maybe we put a short version of it in message (with "Zod" in there to be clear it's a mock API error) and then log the full thing to the console.

Copy link
Collaborator

Choose a reason for hiding this comment

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


// make sure the IP address field has spaces in it
await page.getByLabel('IP Address').fill(' ')

// test that the form can be submitted and a new network interface is created
await sidebar.getByRole('button', { name: 'Add network interface' }).click()
await expect(sidebar).toBeHidden()

// ip address is auto-assigned
await expectRowVisible(page.getByRole('table'), { name: 'nic-2', ip: '123.45.68.8' })
Copy link
Collaborator

Choose a reason for hiding this comment

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

handy row assert util

})