From 59f605653809e4cc9abad73e1ccfdb614510d14f Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 24 May 2024 15:57:02 -0700 Subject: [PATCH 1/2] Filter out committed disks --- .../form/fields/DisksTableField.tsx | 1 + app/forms/disk-attach.tsx | 4 ++- test/e2e/instance-create.e2e.ts | 32 +++++++++++++++++++ test/e2e/instance-disks.e2e.ts | 2 ++ 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/app/components/form/fields/DisksTableField.tsx b/app/components/form/fields/DisksTableField.tsx index 882df8fc78..3013c6e9bd 100644 --- a/app/components/form/fields/DisksTableField.tsx +++ b/app/components/form/fields/DisksTableField.tsx @@ -117,6 +117,7 @@ export function DisksTableField({ onChange([...items, { type: 'attach', ...values }]) setShowDiskAttach(false) }} + disksToFilter={items.filter((i) => i.type === 'attach').map((i) => i.name)} /> )} diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx index f7995fd307..7dc49477f0 100644 --- a/app/forms/disk-attach.tsx +++ b/app/forms/disk-attach.tsx @@ -17,6 +17,7 @@ type AttachDiskProps = { /** If defined, this overrides the usual mutation */ onSubmit: (diskAttach: { name: string }) => void onDismiss: () => void + disksToFilter?: string[] loading?: boolean submitError?: ApiError | null } @@ -28,6 +29,7 @@ type AttachDiskProps = { export function AttachDiskSideModalForm({ onSubmit, onDismiss, + disksToFilter, loading, submitError = null, }: AttachDiskProps) { @@ -39,7 +41,7 @@ export function AttachDiskSideModalForm({ // TODO: error handling const detachedDisks = useApiQuery('diskList', { query: projectSelector }).data?.items.filter( - (d) => d.state.state === 'detached' + (d) => d.state.state === 'detached' && !disksToFilter?.includes(d.name) ) || [] const form = useForm({ defaultValues }) diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts index 4ff6a7b5ed..f531a993d1 100644 --- a/test/e2e/instance-create.e2e.ts +++ b/test/e2e/instance-create.e2e.ts @@ -291,6 +291,38 @@ test('start with an existing disk, but then switch to a silo image', async ({ pa await expectNotVisible(page, ['text=disk-7']) }) +test('additional disks do not list committed disks as available', async ({ page }) => { + await page.goto('/projects/mock-project/instances-new') + + const attachExistingDiskButton = page.getByRole('button', { + name: 'Attach existing disk', + }) + const selectAnOption = page.getByRole('button', { name: 'Select an option' }) + const disk2 = page.getByRole('option', { name: 'disk-2' }) + const disk3 = page.getByRole('option', { name: 'disk-3' }) + const disk4 = page.getByRole('option', { name: 'disk-4' }) + + await attachExistingDiskButton.click() + await selectAnOption.click() + // disk-2 is already attached, so should not be visible in the list + await expect(disk2).toBeHidden() + // disk-3, though, should be present + await expect(disk3).toBeVisible() + await expect(disk4).toBeVisible() + + // select disk-3 and "attach" it to the instance that will be created + await disk3.click() + await page.getByRole('button', { name: 'Attach disk' }).click() + + await attachExistingDiskButton.click() + await selectAnOption.click() + // disk-2 should still be hidden + await expect(disk2).toBeHidden() + // now disk-3 should be hidden as well + await expect(disk3).toBeHidden() + await expect(disk4).toBeVisible() +}) + test('maintains selected values even when changing tabs', async ({ page }) => { const instanceName = 'arch-based-instance' await page.goto('/projects/mock-project/instances-new') diff --git a/test/e2e/instance-disks.e2e.ts b/test/e2e/instance-disks.e2e.ts index 59d1651d1e..373ebbb5ad 100644 --- a/test/e2e/instance-disks.e2e.ts +++ b/test/e2e/instance-disks.e2e.ts @@ -58,6 +58,8 @@ test('Attach disk', async ({ page }) => { await expectVisible(page, ['role=dialog >> text="Disk name is required"']) await page.click('role=button[name*="Disk name"]') + // disk-1 is already attached, so should not be visible in the list + await expectNotVisible(page, ['role=option[name="disk-1"]']) await expectVisible(page, ['role=option[name="disk-3"]', 'role=option[name="disk-4"]']) await page.click('role=option[name="disk-3"]') From e93fdaa4c34e4b53fed7671c90fcea4a2ff553fb Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 28 May 2024 08:45:00 -0700 Subject: [PATCH 2/2] Rename prop; add default value --- app/components/form/fields/DisksTableField.tsx | 2 +- app/forms/disk-attach.tsx | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/components/form/fields/DisksTableField.tsx b/app/components/form/fields/DisksTableField.tsx index 3013c6e9bd..f8389ee650 100644 --- a/app/components/form/fields/DisksTableField.tsx +++ b/app/components/form/fields/DisksTableField.tsx @@ -117,7 +117,7 @@ export function DisksTableField({ onChange([...items, { type: 'attach', ...values }]) setShowDiskAttach(false) }} - disksToFilter={items.filter((i) => i.type === 'attach').map((i) => i.name)} + diskNamesToExclude={items.filter((i) => i.type === 'attach').map((i) => i.name)} /> )} diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx index 7dc49477f0..931346f574 100644 --- a/app/forms/disk-attach.tsx +++ b/app/forms/disk-attach.tsx @@ -17,7 +17,7 @@ type AttachDiskProps = { /** If defined, this overrides the usual mutation */ onSubmit: (diskAttach: { name: string }) => void onDismiss: () => void - disksToFilter?: string[] + diskNamesToExclude?: string[] loading?: boolean submitError?: ApiError | null } @@ -29,7 +29,7 @@ type AttachDiskProps = { export function AttachDiskSideModalForm({ onSubmit, onDismiss, - disksToFilter, + diskNamesToExclude = [], loading, submitError = null, }: AttachDiskProps) { @@ -41,7 +41,7 @@ export function AttachDiskSideModalForm({ // TODO: error handling const detachedDisks = useApiQuery('diskList', { query: projectSelector }).data?.items.filter( - (d) => d.state.state === 'detached' && !disksToFilter?.includes(d.name) + (d) => d.state.state === 'detached' && !diskNamesToExclude.includes(d.name) ) || [] const form = useForm({ defaultValues })