Filter out committed disks (#2262)

charliepark · web-flow · commit ab2939bfa5f3 · 2024-05-28T15:52:29.000Z
diff --git 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)
           }}
+          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
@@ -17,6 +17,7 @@ type AttachDiskProps = {
   /** If defined, this overrides the usual mutation */
   onSubmit: (diskAttach: { name: string }) => void
   onDismiss: () => void
+  diskNamesToExclude?: string[]
   loading?: boolean
   submitError?: ApiError | null
 }
@@ -28,6 +29,7 @@ type AttachDiskProps = {
 export function AttachDiskSideModalForm({
   onSubmit,
   onDismiss,
+  diskNamesToExclude = [],
   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' && !diskNamesToExclude.includes(d.name)
     ) || []
 
   const form = useForm({ defaultValues })
diff --git 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
@@ -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"]')
 
