-
Notifications
You must be signed in to change notification settings - Fork 15
Convert instance form #760
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
Changes from all commits
351f659
914efff
e8d1647
fa4b299
6399c8d
73fe1a0
cb8e0a1
0c5c1d3
65f0eae
5f20f32
a739175
879dffd
2cc26a6
54dcc28
9a51f08
914bb3d
a68dfb0
24b0daa
e91aa2d
1c7660c
2478899
fffc858
51f1d87
a5a03d0
bbba80f
4595486
f2d8057
2a11866
4b77762
e4c0c75
880ba88
1e0ee98
92a59b6
45bcd88
89a86d8
a21e9b5
f64f84f
89ced0d
18e222f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| import React, { useState } from 'react' | ||
| import { useField } from 'formik' | ||
| import { CreateDiskForm } from 'app/forms/disk-create' | ||
| import { AttachDiskForm } from 'app/forms/disk-attach' | ||
| import { | ||
| Button, | ||
| Error16Icon, | ||
| FieldLabel, | ||
| MiniTable, | ||
| SideModal, | ||
| } from '@oxide/ui' | ||
| import type { FormValues } from '../forms' | ||
|
|
||
| type DiskTableItem = | ||
| | (FormValues<'disk-create'> & { type: 'create' }) | ||
| | (FormValues<'disk-attach'> & { type: 'attach' }) | ||
|
|
||
| export function DisksTableField() { | ||
| const [showDiskCreate, setShowDiskCreate] = useState(false) | ||
| const [showDiskAttach, setShowDiskAttach] = useState(false) | ||
|
|
||
| const [, { value: items = [] }, { setValue: setItems }] = useField< | ||
| DiskTableItem[] | ||
| >({ name: 'disks' }) | ||
|
|
||
| return ( | ||
| <> | ||
| <div className="max-w-lg"> | ||
| <FieldLabel id="new-disks-label">{/* this was empty */}</FieldLabel> | ||
| {!!items.length && ( | ||
| <MiniTable className="mb-4"> | ||
| <MiniTable.Header> | ||
| <MiniTable.HeadCell>Name</MiniTable.HeadCell> | ||
| <MiniTable.HeadCell>Type</MiniTable.HeadCell> | ||
| {/* For remove button */} | ||
| <MiniTable.HeadCell className="w-12" /> | ||
| </MiniTable.Header> | ||
| <MiniTable.Body> | ||
| {items.map((item, index) => ( | ||
| <MiniTable.Row | ||
| tabindex="0" | ||
| aria-rowindex={index + 1} | ||
| aria-label={`Name: ${item.name}, Type: ${item.type}`} | ||
| key={item.name} | ||
| > | ||
| <MiniTable.Cell>{item.name}</MiniTable.Cell> | ||
| <MiniTable.Cell>{item.type}</MiniTable.Cell> | ||
| <MiniTable.Cell> | ||
| <Button | ||
| variant="link" | ||
| onClick={() => | ||
| setItems(items.filter((i) => i.name !== item.name)) | ||
| } | ||
| > | ||
| <Error16Icon title={`remove ${item.name}`} /> | ||
| </Button> | ||
| </MiniTable.Cell> | ||
| </MiniTable.Row> | ||
| ))} | ||
| </MiniTable.Body> | ||
| </MiniTable> | ||
| )} | ||
|
|
||
| <div className="space-x-3"> | ||
| <Button | ||
| variant="secondary" | ||
| size="sm" | ||
| onClick={() => setShowDiskCreate(true)} | ||
| > | ||
| Create new disk | ||
| </Button> | ||
| <Button | ||
| variant="secondary" | ||
| size="sm" | ||
| onClick={() => setShowDiskAttach(true)} | ||
| > | ||
| Attach existing disk | ||
| </Button> | ||
| </div> | ||
| </div> | ||
|
|
||
| <SideModal | ||
| id="create-disk-modal" | ||
| isOpen={showDiskCreate} | ||
| onDismiss={() => setShowDiskCreate(false)} | ||
| > | ||
| <CreateDiskForm | ||
| onSubmit={(values) => { | ||
| setItems([...items, { type: 'create', ...values }]) | ||
| setShowDiskCreate(false) | ||
| }} | ||
| /> | ||
| </SideModal> | ||
| <SideModal | ||
| id="attach-disk-modal" | ||
| isOpen={showDiskAttach} | ||
| onDismiss={() => setShowDiskAttach(false)} | ||
| > | ||
| <AttachDiskForm | ||
| onSubmit={(values) => { | ||
| setItems([...items, { type: 'attach', ...values }]) | ||
| setShowDiskAttach(false) | ||
| }} | ||
| /> | ||
| </SideModal> | ||
| </> | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import { test, expect } from '@playwright/test' | ||
|
|
||
| test.describe('Instance Create Form', () => { | ||
| test('can invoke instance create form from instances page', async ({ | ||
| page, | ||
| }) => { | ||
| await page.goto('/orgs/maze-war/projects/mock-project/instances') | ||
| await page.locator('text="New Instance"').click() | ||
| await expect(page.locator('h1:has-text("Create instance")')).toBeVisible() | ||
|
|
||
| await page.fill('input[name=name]', 'mock-instance') | ||
| await page.locator('.ox-radio-card').nth(0).click() | ||
|
|
||
| await page.locator('input[value=ubuntu] ~ .ox-radio-card').click() | ||
|
|
||
| await page.locator('button:has-text("Create instance")').click() | ||
|
|
||
| await page.waitForNavigation() | ||
|
|
||
| expect(page.url()).toContain( | ||
| '/orgs/maze-war/projects/mock-project/instances/mock-instance' | ||
| ) | ||
|
|
||
| await expect(page.locator('h1:has-text("mock-instance")')).toBeVisible() | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import { Form, NameField } from '@oxide/form' | ||
| import React from 'react' | ||
| import type { Disk } from '@oxide/api' | ||
| import { useApiMutation, useApiQueryClient } from '@oxide/api' | ||
| import { invariant } from '@oxide/util' | ||
| import { useParams } from 'app/hooks' | ||
| import type { PrebuiltFormProps } from 'app/forms' | ||
|
|
||
| const values = { | ||
| name: '', | ||
| } | ||
|
|
||
| export function AttachDiskForm({ | ||
| id = 'form-disk-attach', | ||
| title = 'Attach Disk', | ||
| initialValues = values, | ||
| onSubmit, | ||
| onSuccess, | ||
| onError, | ||
| ...props | ||
| }: PrebuiltFormProps<typeof values, Disk>) { | ||
| const queryClient = useApiQueryClient() | ||
| const pathParams = useParams('orgName', 'projectName') | ||
|
|
||
| const attachDisk = useApiMutation('instanceDisksAttach', { | ||
| onSuccess(data) { | ||
| const { instanceName, ...others } = pathParams | ||
| invariant(instanceName, 'instanceName is required') | ||
| queryClient.invalidateQueries('instanceDisksGet', { | ||
| instanceName, | ||
| ...others, | ||
| }) | ||
| onSuccess?.(data) | ||
| }, | ||
| onError, | ||
| }) | ||
|
|
||
| return ( | ||
| <Form | ||
| id={id} | ||
| title={title} | ||
| initialValues={initialValues} | ||
| onSubmit={ | ||
| onSubmit || | ||
| (({ name }) => { | ||
| const { instanceName, ...others } = pathParams | ||
| invariant(instanceName, 'instanceName is required') | ||
| attachDisk.mutate({ | ||
| instanceName, | ||
| ...others, | ||
| body: { name }, | ||
| }) | ||
| }) | ||
| } | ||
| mutation={attachDisk} | ||
| {...props} | ||
| > | ||
| <NameField id="form-disk-attach-name" label="Disk name" /> | ||
| <Form.Actions> | ||
| <Form.Submit>{title}</Form.Submit> | ||
| <Form.Cancel /> | ||
| </Form.Actions> | ||
| </Form> | ||
| ) | ||
| } | ||
|
|
||
| export default AttachDiskForm | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ import babel from '@babel/core' | |
| import { traverse } from '@babel/core' | ||
| import fs from 'fs/promises' | ||
| import path from 'path' | ||
| import './index' | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without explicitly importing index here vitest doesn't correctly when the file changes. |
||
|
|
||
| test('FormTypes must contain references to all forms', async () => { | ||
| let formIds: string[] = [] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,67 @@ | ||
| // TODO: Make these just be default exports | ||
|
|
||
| import type { CreateSubnetForm } from './subnet-create' | ||
| import type { EditSubnetForm } from './subnet-edit' | ||
| import type { CreateOrgForm } from './org-create' | ||
| import type { CreateDiskForm } from './disk-create' | ||
| import type { CreateProjectForm } from './project-create' | ||
| import type CreateInstanceForm from './instance-create' | ||
| import type AttachDiskForm from './disk-attach' | ||
|
|
||
| import type { FormProps } from '@oxide/form' | ||
| import type { ErrorResponse } from '@oxide/api' | ||
| import type { ComponentType } from 'react' | ||
|
|
||
| /** | ||
| * A map of all existing forms. When a new form is created in the forms directory, a | ||
| * new entry should be added here with the key of the string name of the form's filename | ||
| * and a value of the form's type. There's a test to validate that this happens. | ||
| */ | ||
| export interface FormTypes { | ||
| 'instance-create': typeof CreateInstanceForm | ||
| 'org-create': typeof CreateOrgForm | ||
| 'project-create': typeof CreateProjectForm | ||
| 'disk-attach': typeof AttachDiskForm | ||
| 'disk-create': typeof CreateDiskForm | ||
| 'subnet-create': typeof CreateSubnetForm | ||
| 'subnet-edit': typeof EditSubnetForm | ||
| } | ||
|
|
||
| export type FormValues<K extends keyof FormTypes> = ExtractFormValues< | ||
| FormTypes[K] | ||
| > | ||
|
|
||
| /** | ||
| * A form that's built out ahead of time and intended to be re-used dynamically. Fields | ||
| * that are expected to be provided by default are set to optional. | ||
| */ | ||
| export type PrebuiltFormProps<Values, Data> = Omit< | ||
| Optional< | ||
| FormProps<Values>, | ||
| 'id' | 'title' | 'initialValues' | 'onSubmit' | 'mutation' | ||
| >, | ||
| 'children' | ||
| > & { | ||
| children?: never | ||
| onSuccess?: (data: Data) => void | ||
| onError?: (err: ErrorResponse) => void | ||
| } | ||
|
|
||
| /** | ||
| * A utility type for a prebuilt form that extends another form | ||
| */ | ||
| export type ExtendedPrebuiltFormProps<C, D = void> = C extends ComponentType< | ||
| infer B | ||
| > | ||
| ? // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| B extends PrebuiltFormProps<infer V, any> | ||
| ? PrebuiltFormProps<V, D> | ||
| : never | ||
| : never | ||
|
|
||
| export type ExtractFormValues<C> = C extends ComponentType< | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| PrebuiltFormProps<infer V, any> | ||
| > | ||
| ? V | ||
| : never | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These types are complicated enough that it suggests to me we might be doing something we don't need to be doing, but I'm fine leaving it and mulling that over as we use it |
||
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.
what does it mean to require
instanceNameat submit time but not at render time?