From 776ea6e186f26ea2e5dd37f6a60b55022f19f672 Mon Sep 17 00:00:00 2001
From: Bartosz Leper <bartosz.leper@goteleport.com>
Date: Mon, 9 Dec 2024 15:42:23 +0100
Subject: [PATCH] Review

---
 .../src/Roles/RoleEditor/StandardEditor.tsx   | 107 +++++++-----------
 .../src/Roles/RoleEditor/standardmodel.ts     |   4 +-
 2 files changed, 40 insertions(+), 71 deletions(-)

diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.tsx
index 7580c45d06413..332283e0a57d4 100644
--- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.tsx
+++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor.tsx
@@ -268,7 +268,7 @@ export const StandardEditor = ({
             onChange={setCurrentTab}
           />
         </Box>
-        <div
+        <Box
           id={overviewTabId}
           style={{
             display: currentTab === StandardEditorTab.Overview ? '' : 'none',
@@ -280,8 +280,8 @@ export const StandardEditor = ({
             validation={validation.metadata}
             onChange={metadata => handleChange({ ...roleModel, metadata })}
           />
-        </div>
-        <div
+        </Box>
+        <Box
           id={resourcesTabId}
           style={{
             display: currentTab === StandardEditorTab.Resources ? '' : 'none',
@@ -333,8 +333,8 @@ export const StandardEditor = ({
               </MenuButton>
             </Box>
           </Flex>
-        </div>
-        <div
+        </Box>
+        <Box
           id={accessRulesTabId}
           style={{
             display: currentTab === StandardEditorTab.AccessRules ? '' : 'none',
@@ -346,8 +346,8 @@ export const StandardEditor = ({
             onChange={setRules}
             validation={validation.rules}
           />
-        </div>
-        <div
+        </Box>
+        <Box
           id={optionsTabId}
           style={{
             display: currentTab === StandardEditorTab.Options ? '' : 'none',
@@ -358,7 +358,7 @@ export const StandardEditor = ({
             value={roleModel.options}
             onChange={setOptions}
           />
-        </div>
+        </Box>
       </EditorWrapper>
       <EditorSaveCancelButton
         onSave={() => handleSave()}
@@ -1036,25 +1036,13 @@ function Options({
   const requireMFATypeId = `${id}-require-mfa-type`;
   const createHostUserModeId = `${id}-create-host-user-mode`;
   return (
-    <Box
+    <OptionsGridContainer
       border={1}
       borderColor={theme.colors.interactive.tonal.neutral[0]}
       borderRadius={3}
       p={3}
-      css={`
-        display: grid;
-        grid-template-columns: 1fr 1fr;
-        align-items: baseline;
-        row-gap: ${theme.space[3]}px;
-      `}
     >
-      <H4
-        css={`
-          grid-column: 1/3;
-        `}
-      >
-        Global Settings
-      </H4>
+      <OptionsHeader>Global Settings</OptionsHeader>
 
       <OptionLabel htmlFor={maxSessionTTLId}>Max Session TTL</OptionLabel>
       <Input
@@ -1076,7 +1064,7 @@ function Options({
         }
       />
 
-      <div>Disconnect When Certificate Expires</div>
+      <Box>Disconnect When Certificate Expires</Box>
       <BoolRadioGroup
         name="disconnect-expired-cert"
         value={value.disconnectExpiredCert}
@@ -1092,16 +1080,7 @@ function Options({
         onChange={t => onChange?.({ ...value, requireMFAType: t })}
       />
 
-      <H4
-        css={`
-          grid-column: 1/3;
-          border-top: ${theme.borders[1]}
-            ${theme.colors.interactive.tonal.neutral[0]};
-          padding-top: ${theme.space[3]}px;
-        `}
-      >
-        SSH
-      </H4>
+      <OptionsHeader separator>SSH</OptionsHeader>
 
       <OptionLabel htmlFor={createHostUserModeId}>
         Create Host User Mode
@@ -1114,71 +1093,61 @@ function Options({
         onChange={m => onChange?.({ ...value, createHostUserMode: m })}
       />
 
-      <H4
-        css={`
-          grid-column: 1/3;
-          border-top: ${theme.borders[1]}
-            ${theme.colors.interactive.tonal.neutral[0]};
-          padding-top: ${theme.space[3]}px;
-        `}
-      >
-        Database
-      </H4>
+      <OptionsHeader separator>Database</OptionsHeader>
 
-      <div>Create Database User</div>
+      <Box>Create Database User</Box>
       <BoolRadioGroup
         name="create-db-user"
         value={value.createDBUser}
         onChange={c => onChange({ ...value, createDBUser: c })}
       />
 
-      {/* TODO(bl-nero): a bug in YAML unmarshalling backend breaks this option. Fix it. */}
-      {/* <OptionLabel htmlFor={createDBUserModeId}>
-        Create Database User Mode
-      </OptionLabel>
-      <Select
-        inputId={createDBUserModeId}
-        isDisabled={isProcessing}
-        options={createDBUserModeOptions}
-        value={value.createDBUserMode}
-        onChange={m => onChange?.({ ...value, createDBUserMode: m })}
-      /> */}
+      {/* TODO(bl-nero): a bug in YAML unmarshalling backend breaks the
+          createDBUserMode field. Fix it and add the field here. */}
 
-      <H4
-        css={`
-          grid-column: 1/3;
-          border-top: ${theme.borders[1]}
-            ${theme.colors.interactive.tonal.neutral[0]};
-          padding-top: ${theme.space[3]}px;
-        `}
-      >
-        Desktop
-      </H4>
+      <OptionsHeader separator>Desktop</OptionsHeader>
 
-      <div>Create Desktop User</div>
+      <Box>Create Desktop User</Box>
       <BoolRadioGroup
         name="create-desktop-user"
         value={value.createDesktopUser}
         onChange={c => onChange({ ...value, createDesktopUser: c })}
       />
 
-      <div>Allow Clipboard Sharing</div>
+      <Box>Allow Clipboard Sharing</Box>
       <BoolRadioGroup
         name="desktop-clipboard"
         value={value.desktopClipboard}
         onChange={c => onChange({ ...value, desktopClipboard: c })}
       />
 
-      <div>Allow Directory Sharing</div>
+      <Box>Allow Directory Sharing</Box>
       <BoolRadioGroup
         name="desktop-directory-sharing"
         value={value.desktopDirectorySharing}
         onChange={s => onChange({ ...value, desktopDirectorySharing: s })}
       />
-    </Box>
+    </OptionsGridContainer>
   );
 }
 
+const OptionsGridContainer = styled(Box)`
+  display: grid;
+  grid-template-columns: 1fr 1fr;
+  align-items: baseline;
+  row-gap: ${props => props.theme.space[3]}px;
+`;
+
+const OptionsHeader = styled(H4)<{ separator?: boolean }>`
+  grid-column: 1/3;
+  border-top: ${props =>
+    props.separator
+      ? `${props.theme.borders[1]} ${props.theme.colors.interactive.tonal.neutral[0]}`
+      : 'none'};
+  padding-top: ${props =>
+    props.separator ? `${props.theme.space[3]}px` : '0'};
+`;
+
 function BoolRadioGroup({
   name,
   value,
diff --git a/web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts b/web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts
index e8bbb1576da4d..e55288056a80b 100644
--- a/web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts
+++ b/web/packages/teleport/src/Roles/RoleEditor/standardmodel.ts
@@ -661,7 +661,7 @@ function optionsToModel(options: RoleOptions): {
     record_session,
     ssh_file_copy,
 
-    ...others
+    ...unsupported
   } = options;
 
   const requireMFATypeOption =
@@ -703,7 +703,7 @@ function optionsToModel(options: RoleOptions): {
       requireMFATypeOption === undefined ||
       createHostUserModeOption === undefined ||
       createDBUserModeOption === undefined ||
-      !isEmpty(others),
+      !isEmpty(unsupported),
   };
 }