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

Create and edit VPC subnets and routers #593

Merged
merged 10 commits into from Jan 9, 2022
2 changes: 1 addition & 1 deletion app/pages/project/instances/InstancesPage.tsx
Expand Up @@ -49,7 +49,7 @@ export const InstancesPage = () => {
to={`/orgs/${orgName}/projects/${projectName}/instances/new`}
className={buttonStyle({ size: 'xs', variant: 'dim' })}
>
new instance
New Instance
</Link>
</div>
<Table selectable actions={actions}>
Expand Down
32 changes: 32 additions & 0 deletions app/pages/project/networking/VpcPage/VpcPage.spec.ts
@@ -0,0 +1,32 @@
import {
fireEvent,
// logRoles,
renderAppAt,
screen,
// userEvent,
} from '../../../../test-utils'

describe('VpcPage', () => {
describe('subnets tab', () => {
it('does something', async () => {
renderAppAt('/orgs/maze-war/projects/prod-online/vpcs/default')
screen.getByText('Subnets')
const newSubnet = screen.getByRole('button', { name: 'New Subnet' })
expect(screen.queryByRole('button', { name: 'Create subnet' })).toBeNull()
fireEvent.click(newSubnet)

// const ipv4 = screen.getByRole('textbox', { name: 'IPv4 block' })
// const ipv6 = screen.getByRole('textbox', { name: 'IPv6 block' })
// const name = screen.getByRole('textbox', { name: 'Name' })

// this pisses off testing library, "formik change outside of act()". ugh
// userEvent.type(ipv4, 'new-subnet')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😭


// const form = document.getElementById('create-vpc-subnet-form')!
// screen.debug(form, 10000)
// logRoles(form)

// const submit = screen.getByRole('button', { name: 'Create subnet' })
})
})
})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So this test is kind of wild. It takes a few seconds. I'm not sure how brittle it is, it's pretty generic and would be robust to a lot of insignificant things about the page changing. I have liked these kinds of tests in the past because they recreate the kind of manual testing you would do in the browser to make sure happy paths work. But I've never had a whole app full of them, so I don't know how they scale. Curious how you feel about it @zephraph.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One thing I was thinking while doing the mocks is that MSW would make that part better because when we create the subnet it would actually go into the list of subnets and automatically be returned in the subsequent fetch.

Copy link
Contributor

@zephraph zephraph Jan 7, 2022

Choose a reason for hiding this comment

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

Yeah, I'm not super keen on the fetchMock usage. It's not the end of the word and having a few here and there are okay, but generally this isn't the best practice. It reminds me of a PR I did for auto. There were a lot of usages of mocks for fs.existsSync and fs.readFile that meant you really had to know what the implementation was doing to write the tests. Changing the order of file reads or using a different method could break things. In that PR, I helped move over to mock-fs which actually mocked out the whole filesystem... meaning all the read/writes were the same, we just had to mock out what exists on the disk. This is ultimately what MSW will give us and is a really strong argument for why it's worth investing the time to build that out.

That said, impact over perfection. I'm fine with this as is b/c it gives us more test coverage than we had. It'd just be nice to come back through and clean it up once we have a better primitive in place.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I'd like to leave this in, as it gives me a lot more to work with when I make MSW work in #571. It will be very cool if we can delete the mock lines and have the rest of test work as-is.

107 changes: 107 additions & 0 deletions app/pages/project/networking/VpcPage/modals/create-subnet.tsx
@@ -0,0 +1,107 @@
import React from 'react'
import { Formik, Form } from 'formik'

import { Button, FieldTitle, SideModal, TextField } from '@oxide/ui'
import { useApiMutation, useApiQueryClient } from '@oxide/api'

type Props = {
isOpen: boolean
onDismiss: () => void
orgName: string
projectName: string
vpcName: string
}

export function CreateVpcSubnetModal({
isOpen,
onDismiss,
orgName,
projectName,
vpcName,
}: Props) {
const parentIds = {
organizationName: orgName,
projectName,
vpcName,
}
const queryClient = useApiQueryClient()
const createSubnet = useApiMutation('vpcSubnetsPost', {
onSuccess() {
queryClient.invalidateQueries('vpcSubnetsGet', parentIds)
onDismiss()
},
})
const formId = 'create-vpc-subnet-form'
return (
<SideModal
id="create-vpc-subnet-modal"
title="Create subnet"
isOpen={isOpen}
onDismiss={onDismiss}
>
<Formik
initialValues={{
name: '',
description: '',
ipv4Block: '',
ipv6Block: '',
}}
onSubmit={({ name, description, ipv4Block, ipv6Block }) => {
createSubnet.mutate({
...parentIds,
// XXX body is optional. useApiMutation should be smarter and require body when it's required
body: {
name,
description,
// TODO: validate these client-side using the patterns. sadly non-trivial
ipv4Block: ipv4Block || null,
ipv6Block: ipv6Block || null,
},
})
}}
>
<Form id={formId}>
<SideModal.Section>
<div className="space-y-0.5">
<FieldTitle htmlFor="subnet-ipv4-block" tip="TBA">
IPv4 block
</FieldTitle>
<TextField id="subnet-ipv4-block" name="ipv4Block" />
</div>
<div className="space-y-0.5">
<FieldTitle htmlFor="subnet-ipv6-block" tip="TBA">
IPv6 block
</FieldTitle>
<TextField id="subnet-ipv6-block" name="ipv6Block" />
</div>
</SideModal.Section>
<SideModal.Section className="border-t">
<div className="space-y-0.5">
<FieldTitle htmlFor="subnet-name" tip="The name of the subnet">
Name
</FieldTitle>
<TextField id="subnet-name" name="name" />
</div>
<div className="space-y-0.5">
<FieldTitle
htmlFor="subnet-description"
tip="A description for the subnet"
>
Description {/* TODO: indicate optional */}
</FieldTitle>
<TextField id="subnet-description" name="description" />
</div>
</SideModal.Section>
</Form>
</Formik>
<SideModal.Footer>
<Button variant="dim" className="mr-2.5" onClick={onDismiss}>
Cancel
</Button>
<Button form={formId} type="submit">
Create subnet
</Button>
</SideModal.Footer>
</SideModal>
)
}
110 changes: 110 additions & 0 deletions app/pages/project/networking/VpcPage/modals/edit-subnet.tsx
@@ -0,0 +1,110 @@
import React from 'react'
import { Formik, Form } from 'formik'

import { Button, FieldTitle, SideModal, TextField } from '@oxide/ui'
import type { VpcSubnet } from '@oxide/api'
import { useApiMutation, useApiQueryClient } from '@oxide/api'

type Props = {
onDismiss: () => void
orgName: string
projectName: string
vpcName: string
originalSubnet: VpcSubnet | null
}

export function EditVpcSubnetModal({
onDismiss,
orgName,
projectName,
vpcName,
originalSubnet,
}: Props) {
const parentIds = {
organizationName: orgName,
projectName,
vpcName,
}
const queryClient = useApiQueryClient()
const updateSubnet = useApiMutation('vpcSubnetsPutSubnet', {
onSuccess() {
queryClient.invalidateQueries('vpcSubnetsGet', parentIds)
onDismiss()
},
})

if (!originalSubnet) return null

const formId = 'edit-vpc-subnet-form'
return (
<SideModal
id="edit-vpc-subnet-modal"
title="Edit subnet"
onDismiss={onDismiss}
>
<Formik
initialValues={{
name: originalSubnet.identity.name,
description: originalSubnet.identity.description,
ipv4Block: originalSubnet.ipv4_block || '',
ipv6Block: originalSubnet.ipv6_block || '',
}}
onSubmit={({ name, description, ipv4Block, ipv6Block }) => {
updateSubnet.mutate({
...parentIds,
subnetName: originalSubnet.identity.name,
body: {
name,
description,
// TODO: validate these client-side using the patterns. sadly non-trivial
ipv4Block: ipv4Block || null,
ipv6Block: ipv6Block || null,
},
})
}}
>
<Form id={formId}>
<SideModal.Section>
<div className="space-y-0.5">
<FieldTitle htmlFor="subnet-ipv4-block" tip="TBA">
IPv4 block
</FieldTitle>
<TextField id="subnet-ipv4-block" name="ipv4Block" />
</div>
<div className="space-y-0.5">
<FieldTitle htmlFor="subnet-ipv6-block" tip="TBA">
IPv6 block
</FieldTitle>
<TextField id="subnet-ipv6-block" name="ipv6Block" />
</div>
</SideModal.Section>
<SideModal.Section className="border-t">
<div className="space-y-0.5">
<FieldTitle htmlFor="subnet-name" tip="The name of the subnet">
Name
</FieldTitle>
<TextField id="subnet-name" name="name" />
</div>
<div className="space-y-0.5">
<FieldTitle
htmlFor="subnet-description"
tip="A description for the subnet"
>
Description {/* TODO: indicate optional */}
</FieldTitle>
<TextField id="subnet-description" name="description" />
</div>
</SideModal.Section>
</Form>
</Formik>
<SideModal.Footer>
<Button variant="dim" className="mr-2.5" onClick={onDismiss}>
Cancel
</Button>
<Button form={formId} type="submit">
Update subnet
</Button>
</SideModal.Footer>
</SideModal>
)
}
70 changes: 56 additions & 14 deletions app/pages/project/networking/VpcPage/tabs/VpcSubnetsTab.tsx
@@ -1,29 +1,71 @@
import React from 'react'
import React, { useState } from 'react'
import { useParams } from '../../../../../hooks'
import type { MenuAction } from '@oxide/table'
import { useQueryTable, TwoLineCell, DateCell } from '@oxide/table'
import { Button } from '@oxide/ui'
import { CreateVpcSubnetModal } from '../modals/create-subnet'
import { EditVpcSubnetModal } from '../modals/edit-subnet'
import type { VpcSubnet } from '@oxide/api'

export const VpcSubnetsTab = () => {
const { orgName: organizationName, ...other } = useParams(
const { orgName, projectName, vpcName } = useParams(
'orgName',
'projectName',
'vpcName'
)

const { Table, Column } = useQueryTable('vpcSubnetsGet', {
organizationName,
...other,
organizationName: orgName,
projectName,
vpcName,
})

const [createModalOpen, setCreateModalOpen] = useState(false)
const [editingSubnet, setEditingSubnet] = useState<VpcSubnet | null>(null)

const actions = (subnet: VpcSubnet): MenuAction[] => [
{
label: 'Edit',
onActivate: () => setEditingSubnet(subnet),
// TODO: disable for default? can you edit it?
},
]

return (
<Table selectable>
<Column id="name" accessor="identity.name" />
<Column
id="ip-block"
header="IP Block"
accessor={(vpc) => [vpc.ipv4_block, vpc.ipv6_block]}
cell={TwoLineCell}
/>
<Column id="created" accessor="identity.timeCreated" cell={DateCell} />
</Table>
<>
<div className="mb-3 flex justify-end space-x-4">
<Button
size="xs"
variant="dim"
onClick={() => setCreateModalOpen(true)}
>
New Subnet
</Button>
<CreateVpcSubnetModal
orgName={orgName}
projectName={projectName}
vpcName={vpcName}
isOpen={createModalOpen}
onDismiss={() => setCreateModalOpen(false)}
/>
<EditVpcSubnetModal
orgName={orgName}
projectName={projectName}
vpcName={vpcName}
originalSubnet={editingSubnet} // modal is open if this is non-null
onDismiss={() => setEditingSubnet(null)}
/>
</div>
<Table selectable actions={actions}>
<Column id="name" accessor="identity.name" />
<Column
id="ip-block"
header="IP Block"
accessor={(vpc) => [vpc.ipv4_block, vpc.ipv6_block]}
cell={TwoLineCell}
/>
<Column id="created" accessor="identity.timeCreated" cell={DateCell} />
</Table>
</>
)
}
1 change: 1 addition & 0 deletions app/test-utils.tsx
Expand Up @@ -38,4 +38,5 @@ export const lastPostBody = (mock: FetchMockStatic): any =>
JSON.parse(mock.lastOptions(undefined, 'POST')?.body as unknown as string)

export * from '@testing-library/react'
export { default as userEvent } from '@testing-library/user-event'
export { customRender as render }
15 changes: 12 additions & 3 deletions libs/ui/lib/side-modal/SideModal.tsx
Expand Up @@ -5,6 +5,7 @@ import { Button } from '../button/Button'
import { pluckFirstOfType } from '@oxide/util'
import type { ChildrenProp } from '@oxide/util'
import { Close12Icon } from '../icons'
import cn from 'classnames'

export interface SideModalProps extends DialogProps, ChildrenProp {
id: string
Expand All @@ -27,7 +28,7 @@ export function SideModal({
id={id}
onDismiss={onDismiss}
{...dialogProps}
className="absolute right-0 top-0 bottom-0 w-[32rem] p-0 m-0 flex flex-col justify-between bg-gray-500 border-l border-gray-400"
className="absolute right-0 top-0 bottom-0 w-[32rem] p-0 m-0 flex flex-col justify-between bg-black border-l border-gray-400"
aria-labelledby={titleId}
>
<div
Expand All @@ -51,8 +52,16 @@ export function SideModal({
)
}

SideModal.Section = ({ children }: ChildrenProp) => (
<div className="p-8 space-y-6 border-gray-400">{children}</div>
SideModal.Section = ({
children,
className,
}: {
children: React.ReactNode
className?: string
}) => (
<div className={cn('p-8 space-y-6 border-gray-400', className)}>
{children}
</div>
)

SideModal.Docs = ({ children }: ChildrenProp) => (
Expand Down