From 3d254ec0f8ed67329a01ac2d4c2f57372b5b8ee9 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 17 Jul 2024 14:43:51 -0400 Subject: [PATCH 1/9] Tie rendering logic for admin group name to identity mode --- app/forms/silo-create.tsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/app/forms/silo-create.tsx b/app/forms/silo-create.tsx index 8e2a42ef07..fca91ea639 100644 --- a/app/forms/silo-create.tsx +++ b/app/forms/silo-create.tsx @@ -65,6 +65,7 @@ export function CreateSiloSideModalForm() { }) const form = useForm({ defaultValues }) + const identityMode = form.watch('identityMode') return ( - + {identityMode === 'saml_jit' && ( + + )}
Grant fleet admin role to silo admins From e3a44d7c50bd6f4174825e3d9bb4cb17c1a0f7c5 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 17 Jul 2024 17:20:33 -0400 Subject: [PATCH 2/9] Put more of form behind conditional render; update test --- app/forms/silo-create.tsx | 64 ++++++++++++++++++++++++++++----------- test/e2e/silos.e2e.ts | 4 ++- 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/app/forms/silo-create.tsx b/app/forms/silo-create.tsx index fca91ea639..6ada380671 100644 --- a/app/forms/silo-create.tsx +++ b/app/forms/silo-create.tsx @@ -5,6 +5,7 @@ * * Copyright Oxide Computer Company */ +import { useEffect } from 'react' import { useNavigate } from 'react-router-dom' import { useApiMutation, useApiQueryClient, type SiloCreate } from '@oxide/api' @@ -66,7 +67,21 @@ export function CreateSiloSideModalForm() { const form = useForm({ defaultValues }) const identityMode = form.watch('identityMode') - + const adminGroupName = form.watch('adminGroupName') + // Clear the adminGroupName if the user selects the "local only" identity mode + useEffect(() => { + if (identityMode === 'local_only') { + form.setValue('adminGroupName', '') + } + }, [identityMode, form]) + // Clear the role assignment checkboxes if the adminGroupName is deleted + const fleetRolesDisabled = !adminGroupName + useEffect(() => { + if (fleetRolesDisabled) { + form.setValue('siloAdminGetsFleetAdmin', false) + form.setValue('siloViewerGetsFleetViewer', false) + } + }, [fleetRolesDisabled, form]) return ( {identityMode === 'saml_jit' && ( - + <> + + This group will be created and granted the Silo Admin role. +
A name is required to assign fleet roles. + + } + control={form.control} + /> +
+ + Grant fleet admin role to silo admins + + + Grant fleet viewer role to silo viewers + +
+ )} -
- - Grant fleet admin role to silo admins - -
-
- - Grant fleet viewer role to silo viewers - -
diff --git a/test/e2e/silos.e2e.ts b/test/e2e/silos.e2e.ts index 43b1154c82..9ef4286d67 100644 --- a/test/e2e/silos.e2e.ts +++ b/test/e2e/silos.e2e.ts @@ -39,6 +39,8 @@ test('Create silo', async ({ page }) => { await expect(discoverable).toBeChecked() await discoverable.click() await page.getByRole('radio', { name: 'Local only' }).click() + await expect(page.getByRole('textbox', { name: 'Admin group name' })).toBeHidden() + await page.getByRole('radio', { name: 'SAML' }).click() await page.getByRole('textbox', { name: 'Admin group name' }).fill('admins') await page.getByRole('checkbox', { name: 'Grant fleet admin' }).click() await page.getByRole('textbox', { name: 'CPU quota' }).fill('30') @@ -99,7 +101,7 @@ test('Create silo', async ({ page }) => { await expectRowVisible(table, { name: 'other-silo', description: 'definitely a silo', - 'Identity mode': 'local only', + 'Identity mode': 'saml jit', // discoverable: 'false', }) const otherSiloCell = page.getByRole('cell', { name: 'other-silo' }) From 43f9ecb7160f2d627db7d03d2933370bcc30e7e4 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 17 Jul 2024 17:57:40 -0400 Subject: [PATCH 3/9] Copy adjustment --- app/forms/silo-create.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/forms/silo-create.tsx b/app/forms/silo-create.tsx index 6ada380671..a677b2a7de 100644 --- a/app/forms/silo-create.tsx +++ b/app/forms/silo-create.tsx @@ -169,7 +169,8 @@ export function CreateSiloSideModalForm() { description={ <> This group will be created and granted the Silo Admin role. -
A name is required to assign fleet roles. +
+ An admin group name is required to assign fleet roles. } control={form.control} From c285397681e7b3aa1db2bd8af0691ee0bdd46670 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 17 Jul 2024 19:18:35 -0400 Subject: [PATCH 4/9] add test assertions to match writeup in PR description --- test/e2e/silos.e2e.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/e2e/silos.e2e.ts b/test/e2e/silos.e2e.ts index 9ef4286d67..5763f37277 100644 --- a/test/e2e/silos.e2e.ts +++ b/test/e2e/silos.e2e.ts @@ -38,9 +38,18 @@ test('Create silo', async ({ page }) => { const discoverable = page.getByRole('checkbox', { name: 'Discoverable' }) await expect(discoverable).toBeChecked() await discoverable.click() + await expect(page.getByRole('textbox', { name: 'Admin group name' })).toBeVisible() + await page.getByRole('textbox', { name: 'Admin group name' }).fill('admins') + await page.getByRole('checkbox', { name: 'Grant fleet admin' }).click() + await expect(page.getByRole('textbox', { name: 'Admin group name' })).toHaveValue( + 'admins' + ) + await expect(page.getByRole('checkbox', { name: 'Grant fleet admin' })).toBeChecked() await page.getByRole('radio', { name: 'Local only' }).click() await expect(page.getByRole('textbox', { name: 'Admin group name' })).toBeHidden() await page.getByRole('radio', { name: 'SAML' }).click() + await expect(page.getByRole('textbox', { name: 'Admin group name' })).toHaveValue('') + await expect(page.getByRole('checkbox', { name: 'Grant fleet admin' })).not.toBeChecked() await page.getByRole('textbox', { name: 'Admin group name' }).fill('admins') await page.getByRole('checkbox', { name: 'Grant fleet admin' }).click() await page.getByRole('textbox', { name: 'CPU quota' }).fill('30') From c4c89b43fd4275518e4d81fcd2dd1d2c88ee9666 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 24 Jul 2024 13:24:39 -0400 Subject: [PATCH 5/9] Move checkboxes back out of hidey-block --- app/forms/silo-create.tsx | 63 ++++++++++++++------------------------- test/e2e/silos.e2e.ts | 3 +- 2 files changed, 23 insertions(+), 43 deletions(-) diff --git a/app/forms/silo-create.tsx b/app/forms/silo-create.tsx index a677b2a7de..f02c915a4f 100644 --- a/app/forms/silo-create.tsx +++ b/app/forms/silo-create.tsx @@ -67,21 +67,12 @@ export function CreateSiloSideModalForm() { const form = useForm({ defaultValues }) const identityMode = form.watch('identityMode') - const adminGroupName = form.watch('adminGroupName') // Clear the adminGroupName if the user selects the "local only" identity mode useEffect(() => { if (identityMode === 'local_only') { form.setValue('adminGroupName', '') } }, [identityMode, form]) - // Clear the role assignment checkboxes if the adminGroupName is deleted - const fleetRolesDisabled = !adminGroupName - useEffect(() => { - if (fleetRolesDisabled) { - form.setValue('siloAdminGetsFleetAdmin', false) - form.setValue('siloViewerGetsFleetViewer', false) - } - }, [fleetRolesDisabled, form]) return ( {identityMode === 'saml_jit' && ( - <> - - This group will be created and granted the Silo Admin role. -
- An admin group name is required to assign fleet roles. - - } - control={form.control} - /> -
- - Grant fleet admin role to silo admins - - - Grant fleet viewer role to silo viewers - -
- + )} +
+ + Grant fleet admin role to silo admins + + + Grant fleet viewer role to silo viewers + +
diff --git a/test/e2e/silos.e2e.ts b/test/e2e/silos.e2e.ts index 5763f37277..445483d49f 100644 --- a/test/e2e/silos.e2e.ts +++ b/test/e2e/silos.e2e.ts @@ -49,9 +49,8 @@ test('Create silo', async ({ page }) => { await expect(page.getByRole('textbox', { name: 'Admin group name' })).toBeHidden() await page.getByRole('radio', { name: 'SAML' }).click() await expect(page.getByRole('textbox', { name: 'Admin group name' })).toHaveValue('') - await expect(page.getByRole('checkbox', { name: 'Grant fleet admin' })).not.toBeChecked() + await expect(page.getByRole('checkbox', { name: 'Grant fleet admin' })).toBeChecked() await page.getByRole('textbox', { name: 'Admin group name' }).fill('admins') - await page.getByRole('checkbox', { name: 'Grant fleet admin' }).click() await page.getByRole('textbox', { name: 'CPU quota' }).fill('30') await page.getByRole('textbox', { name: 'Memory quota' }).fill('58') await page.getByRole('textbox', { name: 'Storage quota' }).fill('735') From 0319829bd944fe1c3ef284b7cb276989fd699d14 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 24 Jul 2024 13:26:51 -0400 Subject: [PATCH 6/9] Remove unneeded classNames --- app/forms/silo-create.tsx | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/app/forms/silo-create.tsx b/app/forms/silo-create.tsx index f02c915a4f..c586ec4fcb 100644 --- a/app/forms/silo-create.tsx +++ b/app/forms/silo-create.tsx @@ -161,18 +161,10 @@ export function CreateSiloSideModalForm() { /> )}
- + Grant fleet admin role to silo admins - + Grant fleet viewer role to silo viewers
From f7feafc23647f0a900db38adde2ade10101743e2 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 24 Jul 2024 14:51:12 -0400 Subject: [PATCH 7/9] Break up sections --- app/forms/silo-create.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/forms/silo-create.tsx b/app/forms/silo-create.tsx index c586ec4fcb..d25271fc2f 100644 --- a/app/forms/silo-create.tsx +++ b/app/forms/silo-create.tsx @@ -21,6 +21,7 @@ import { SideModalForm } from '~/components/form/SideModalForm' import { useForm } from '~/hooks' import { addToast } from '~/stores/toast' import { FormDivider } from '~/ui/lib/Divider' +import { FieldLabel } from '~/ui/lib/FieldLabel' import { Message } from '~/ui/lib/Message' import { pb } from '~/util/path-builder' import { GiB } from '~/util/units' @@ -160,7 +161,11 @@ export function CreateSiloSideModalForm() { control={form.control} /> )} +
+ + Role mapping + Grant fleet admin role to silo admins From 719f24f3b58a46be3d2675f0b63fb8d3edd0157c Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 24 Jul 2024 14:57:12 -0400 Subject: [PATCH 8/9] small spacing below 'Role mapping' label --- app/forms/silo-create.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/forms/silo-create.tsx b/app/forms/silo-create.tsx index d25271fc2f..7240a27a00 100644 --- a/app/forms/silo-create.tsx +++ b/app/forms/silo-create.tsx @@ -163,7 +163,7 @@ export function CreateSiloSideModalForm() { )}
- + Role mapping From 8744e7ccce140ebb79515981387142aef9bfe4ec Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 24 Jul 2024 15:20:34 -0500 Subject: [PATCH 9/9] tweak spacing slightly --- app/forms/silo-create.tsx | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/app/forms/silo-create.tsx b/app/forms/silo-create.tsx index 7240a27a00..f1a5fc5a00 100644 --- a/app/forms/silo-create.tsx +++ b/app/forms/silo-create.tsx @@ -162,16 +162,18 @@ export function CreateSiloSideModalForm() { /> )} -
- +
+ Role mapping - - Grant fleet admin role to silo admins - - - Grant fleet viewer role to silo viewers - +
+ + Grant fleet admin role to silo admins + + + Grant fleet viewer role to silo viewers + +