Create and edit VPC subnets and routers #593
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| import { | ||
| fireEvent, | ||
| lastPostBody, | ||
| renderAppAt, | ||
| screen, | ||
| userEvent, | ||
| waitForElementToBeRemoved, | ||
| } from '../../../../test-utils' | ||
| import fetchMock from 'fetch-mock' | ||
| import { | ||
| org, | ||
| project, | ||
| vpc, | ||
| vpcSubnet, | ||
| vpcSubnet2, | ||
| vpcSubnets, | ||
| } from '@oxide/api-mocks' | ||
|
|
||
| const vpcUrl = `/api/organizations/${org.name}/projects/${project.name}/vpcs/default` | ||
| const subnetsUrl = `${vpcUrl}/subnets` | ||
| const getSubnetsUrl = `${subnetsUrl}?limit=10` | ||
|
|
||
| describe('VpcPage', () => { | ||
| describe('subnets tab', () => { | ||
| it('creating a subnet works', async () => { | ||
| fetchMock.get(vpcUrl, { status: 200, body: vpc }) | ||
| fetchMock.getOnce(getSubnetsUrl, { status: 200, body: vpcSubnets }) | ||
| const postMock = fetchMock.postOnce(subnetsUrl, { | ||
| status: 201, | ||
| body: vpcSubnet2, | ||
| }) | ||
|
|
||
| renderAppAt('/orgs/mock-org/projects/mock-project/vpcs/default') | ||
| screen.getByText('Subnets') | ||
|
|
||
| // wait for subnet to show up in the table | ||
| await screen.findByRole('cell', { name: vpcSubnet.identity.name }) | ||
|
|
||
| // modal is not already open | ||
| expect(screen.queryByRole('dialog', { name: 'Create subnet' })).toBeNull() | ||
|
|
||
| // click button to open modal | ||
| fireEvent.click(screen.getByRole('button', { name: 'New subnet' })) | ||
|
|
||
| // modal is open | ||
| screen.getByRole('dialog', { name: 'Create subnet' }) | ||
|
|
||
| const ipv4 = screen.getByRole('textbox', { name: 'IPv4 block' }) | ||
| userEvent.type(ipv4, '1.1.1.2/24') | ||
|
|
||
| const name = screen.getByRole('textbox', { name: 'Name' }) | ||
| userEvent.type(name, 'mock-subnet-2') | ||
|
|
||
| // override the subnets GET to include both subnets | ||
| fetchMock.getOnce( | ||
| getSubnetsUrl, | ||
| { | ||
| status: 200, | ||
| body: { items: [vpcSubnet, vpcSubnet2] }, | ||
| }, | ||
| { overwriteRoutes: true } | ||
| ) | ||
|
|
||
| // submit the form | ||
| fireEvent.click(screen.getByRole('button', { name: 'Create subnet' })) | ||
|
|
||
| // wait for modal to close | ||
| await waitForElementToBeRemoved(() => | ||
| screen.queryByRole('dialog', { name: 'Create subnet' }) | ||
| ) | ||
|
|
||
| // it posted the form | ||
| expect(lastPostBody(postMock)).toEqual({ | ||
| ipv4Block: '1.1.1.2/24', | ||
| ipv6Block: null, | ||
| name: 'mock-subnet-2', | ||
| description: '', | ||
| }) | ||
|
|
||
| // table should refetch and now include second subnet | ||
| screen.getByRole('cell', { name: vpcSubnet.identity.name }) | ||
| screen.getByRole('cell', { name: vpcSubnet2.identity.name }) | ||
| }) | ||
| }) | ||
| }) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,170 @@ | ||
| import React from 'react' | ||
| import { Formik, Form } from 'formik' | ||
|
|
||
| import { Button, FieldTitle, SideModal, TextField } from '@oxide/ui' | ||
| import type { VpcRouter, ErrorResponse } from '@oxide/api' | ||
| import { useApiMutation, useApiQueryClient } from '@oxide/api' | ||
| import { getServerError } from '../../../../../util/errors' | ||
|
|
||
| // this will get a lot more interesting once the API is updated to allow us to | ||
| // put rules in the router, which is the whole point of a router | ||
|
|
||
| type FormProps = { | ||
| error: ErrorResponse | null | ||
| id: string | ||
| } | ||
|
|
||
| // the moment the two forms diverge, inline them rather than introducing BS | ||
| // props here | ||
| const CommonForm = ({ error, id }: FormProps) => ( | ||
| <Form id={id}> | ||
| <SideModal.Section className="border-t"> | ||
| <div className="space-y-0.5"> | ||
| <FieldTitle htmlFor="router-name" tip="The name of the router"> | ||
| Name | ||
| </FieldTitle> | ||
| <TextField id="router-name" name="name" /> | ||
| </div> | ||
| <div className="space-y-0.5"> | ||
| <FieldTitle | ||
| htmlFor="router-description" | ||
| tip="A description for the router" | ||
| > | ||
| Description {/* TODO: indicate optional */} | ||
| </FieldTitle> | ||
| <TextField id="router-description" name="description" /> | ||
| </div> | ||
| </SideModal.Section> | ||
| <SideModal.Section> | ||
| <div className="text-red-500">{getServerError(error)}</div> | ||
| </SideModal.Section> | ||
| </Form> | ||
| ) | ||
|
|
||
| type CreateProps = { | ||
| isOpen: boolean | ||
| onDismiss: () => void | ||
| orgName: string | ||
| projectName: string | ||
| vpcName: string | ||
| } | ||
|
|
||
| export function CreateVpcRouterModal({ | ||
| isOpen, | ||
| onDismiss, | ||
| orgName, | ||
| projectName, | ||
| vpcName, | ||
| }: CreateProps) { | ||
| const parentIds = { orgName, projectName, vpcName } | ||
| const queryClient = useApiQueryClient() | ||
|
|
||
| function dismiss() { | ||
| createRouter.reset() | ||
| onDismiss() | ||
| } | ||
|
|
||
| const createRouter = useApiMutation('vpcRoutersPost', { | ||
| onSuccess() { | ||
| queryClient.invalidateQueries('vpcRoutersGet', parentIds) | ||
| dismiss() | ||
| }, | ||
| }) | ||
|
|
||
| const formId = 'create-vpc-router-form' | ||
|
|
||
| return ( | ||
| <SideModal | ||
| id="create-vpc-router-modal" | ||
| title="Create router" | ||
| isOpen={isOpen} | ||
| onDismiss={dismiss} | ||
| > | ||
| <Formik | ||
| initialValues={{ name: '', description: '' }} | ||
| onSubmit={({ name, description }) => { | ||
| createRouter.mutate({ | ||
| ...parentIds, | ||
| body: { name, description }, | ||
| }) | ||
| }} | ||
| > | ||
| <CommonForm id={formId} error={createRouter.error} /> | ||
| </Formik> | ||
| <SideModal.Footer> | ||
| <Button variant="dim" className="mr-2.5" onClick={dismiss}> | ||
| Cancel | ||
| </Button> | ||
| <Button form={formId} type="submit"> | ||
| Create router | ||
| </Button> | ||
| </SideModal.Footer> | ||
| </SideModal> | ||
| ) | ||
| } | ||
|
|
||
| type EditProps = { | ||
| onDismiss: () => void | ||
| orgName: string | ||
| projectName: string | ||
| vpcName: string | ||
| originalRouter: VpcRouter | null | ||
| } | ||
|
|
||
| export function EditVpcRouterModal({ | ||
| onDismiss, | ||
| orgName, | ||
| projectName, | ||
| vpcName, | ||
| originalRouter, | ||
| }: EditProps) { | ||
| const parentIds = { orgName, projectName, vpcName } | ||
| const queryClient = useApiQueryClient() | ||
|
|
||
| function dismiss() { | ||
| updateRouter.reset() | ||
| onDismiss() | ||
| } | ||
|
|
||
| const updateRouter = useApiMutation('vpcRoutersPutRouter', { | ||
| onSuccess() { | ||
| queryClient.invalidateQueries('vpcRoutersGet', parentIds) | ||
| dismiss() | ||
| }, | ||
| }) | ||
|
|
||
| if (!originalRouter) return null | ||
|
|
||
| const formId = 'edit-vpc-router-form' | ||
| return ( | ||
| <SideModal | ||
| id="edit-vpc-router-modal" | ||
| title="Edit router" | ||
| onDismiss={dismiss} | ||
| > | ||
| <Formik | ||
| initialValues={{ | ||
| name: originalRouter.identity.name, | ||
| description: originalRouter.identity.description, | ||
| }} | ||
| onSubmit={({ name, description }) => { | ||
| updateRouter.mutate({ | ||
| ...parentIds, | ||
| routerName: originalRouter.identity.name, | ||
| body: { name, description }, | ||
| }) | ||
| }} | ||
| > | ||
| <CommonForm id={formId} error={updateRouter.error} /> | ||
| </Formik> | ||
| <SideModal.Footer> | ||
| <Button variant="dim" className="mr-2.5" onClick={dismiss}> | ||
| Cancel | ||
| </Button> | ||
| <Button form={formId} type="submit"> | ||
| Update router | ||
| </Button> | ||
| </SideModal.Footer> | ||
| </SideModal> | ||
| ) | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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
fetchMockusage. 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 forfs.existsSyncandfs.readFilethat 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 tomock-fswhich 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.
There was a problem hiding this comment.
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.