Address comments

evergreen-ci · Sep 22, 2023 · 12bd557 · 12bd557
1 parent b0a3238
commit 12bd557
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 40 deletions.
diff --git a/cypress/integration/distroSettings/provider_section.ts b/cypress/integration/distroSettings/provider_section.ts
@@ -92,15 +92,16 @@ describe("provider section", () => {
     it("shows and hides fields correctly", () => {
       // Fleet options.
       cy.getInputByLabel("Fleet Instance Type").contains("On-demand");
-      cy.dataCy("capacity-optimization").should("not.exist");
+      cy.contains("Capacity optimization").should("not.exist");
 
       cy.selectLGOption("Fleet Instance Type", "Spot");
-      cy.dataCy("use-capacity-optimization").should("exist");
+      cy.contains("Capacity optimization").should("be.visible");
 
       // VPC options.
+      cy.dataCy("use-vpc").scrollIntoView();
       cy.dataCy("use-vpc").should("be.checked");
-      cy.contains("Default VPC Subnet ID").should("exist");
-      cy.contains("VPC Subnet Prefix").should("exist");
+      cy.contains("Default VPC Subnet ID").should("be.visible");
+      cy.contains("VPC Subnet Prefix").should("be.visible");
 
       cy.dataCy("use-vpc").uncheck({ force: true });
       cy.contains("Default VPC Subnet ID").should("not.exist");
@@ -111,16 +112,14 @@ describe("provider section", () => {
       cy.dataCy("provider-select").contains("EC2 Fleet");
 
       // Correct section is displayed.
-      cy.dataCy("ec2-fleet-provider-settings").should("exist");
-      cy.dataCy("ec2-fleet-provider-settings")
-        .children()
-        .should("have.length", 1);
+      cy.dataCy("ec2-fleet-provider-settings").should("be.visible");
       cy.dataCy("region-select").contains("us-east-1");
 
       // Change field values.
       cy.selectLGOption("Region", "us-west-1");
-      cy.getInputByLabel("SSH Key Name").clear();
-      cy.getInputByLabel("SSH Key Name").type("my ssh key");
+      cy.getInputByLabel("SSH Key Name").as("keyNameInput");
+      cy.get("@keyNameInput").clear();
+      cy.get("@keyNameInput").type("my ssh key");
       cy.selectLGOption("Fleet Instance Type", "Spot");
       cy.contains("button", "Add mount point").click();
       cy.getInputByLabel("Device Name").type("device name");
@@ -130,8 +129,8 @@ describe("provider section", () => {
 
       // Revert fields to original values.
       cy.selectLGOption("Region", "us-east-1");
-      cy.getInputByLabel("SSH Key Name").clear();
-      cy.getInputByLabel("SSH Key Name").type("mci");
+      cy.get("@keyNameInput").clear();
+      cy.get("@keyNameInput").type("mci");
       cy.selectLGOption("Fleet Instance Type", "On-demand");
       cy.dataCy("mount-points").within(() => {
         cy.dataCy("delete-item-button").click();
@@ -141,16 +140,10 @@ describe("provider section", () => {
     });
 
     it("can add and delete region settings", () => {
-      cy.dataCy("ec2-fleet-provider-settings").should("exist");
-      cy.dataCy("ec2-fleet-provider-settings")
-        .children()
-        .should("have.length", 1);
+      cy.dataCy("ec2-fleet-provider-settings").should("be.visible");
 
       // Add item for new region.
       cy.contains("button", "Add region settings").click();
-      cy.dataCy("ec2-fleet-provider-settings")
-        .children()
-        .should("have.length", 2);
       cy.contains("button", "Add region settings").should("not.exist");
 
       // Save new region.
@@ -167,10 +160,7 @@ describe("provider section", () => {
       save();
       cy.validateToast("success");
 
-      cy.contains("button", "Add region settings").should("exist");
-      cy.dataCy("ec2-fleet-provider-settings")
-        .children()
-        .should("have.length", 1);
+      cy.contains("button", "Add region settings").should("be.visible");
     });
   });
 });
diff --git a/src/pages/distroSettings/tabs/ProviderTab/ProviderTab.tsx b/src/pages/distroSettings/tabs/ProviderTab/ProviderTab.tsx
@@ -1,7 +1,7 @@
 import { useMemo } from "react";
 import { useQuery } from "@apollo/client";
 import { AwsRegionsQuery, AwsRegionsQueryVariables } from "gql/generated/types";
-import { GET_AWS_REGIONS } from "gql/queries";
+import { AWS_REGIONS } from "gql/queries";
 import { useSpruceConfig } from "hooks";
 import { useDistroSettingsContext } from "pages/distroSettings/Context";
 import { omitTypename } from "utils/string";
@@ -28,10 +28,12 @@ export const ProviderTab: React.FC<TabProps> = ({ distro, distroData }) => {
   } = getTab(WritableDistroSettingsTabs.Provider);
 
   const { data: awsData } = useQuery<AwsRegionsQuery, AwsRegionsQueryVariables>(
-    GET_AWS_REGIONS
+    AWS_REGIONS
   );
   const { awsRegions } = awsData || {};
-  const fleetRegions = formData?.ec2FleetProviderSettings?.map((p) => p.region);
+  const configuredRegions = formData?.ec2FleetProviderSettings?.map(
+    (p) => p.region
+  );
 
   const { containerPools } = useSpruceConfig();
   const { pools } = containerPools || {};
@@ -48,9 +50,9 @@ export const ProviderTab: React.FC<TabProps> = ({ distro, distroData }) => {
         pools: pools || [],
         poolMappingInfo,
         awsRegions: awsRegions || [],
-        fleetRegions: fleetRegions || [],
+        configuredRegions: configuredRegions || [],
       }),
-    [pools, poolMappingInfo, awsRegions, fleetRegions]
+    [pools, poolMappingInfo, awsRegions, configuredRegions]
   );
 
   return (

diff --git a/src/pages/distroSettings/tabs/ProviderTab/getFormSchema.ts b/src/pages/distroSettings/tabs/ProviderTab/getFormSchema.ts
@@ -5,6 +5,7 @@ import {
   AccordionFieldTemplate,
 } from "components/SpruceForm/FieldTemplates";
 import { STANDARD_FIELD_WIDTH } from "components/SpruceForm/utils";
+import { size } from "constants/tokens";
 import { Provider, ContainerPool } from "gql/generated/types";
 import {
   dockerProviderSettings,
@@ -14,14 +15,14 @@ import {
 
 export const getFormSchema = ({
   awsRegions,
-  fleetRegions,
+  configuredRegions,
   poolMappingInfo,
   pools,
 }: {
   awsRegions: string[];
+  configuredRegions: string[];
   poolMappingInfo: string;
   pools: ContainerPool[];
-  fleetRegions: string[];
 }): ReturnType<GetFormSchema> => ({
   fields: {},
   schema: {
@@ -107,8 +108,8 @@ export const getFormSchema = ({
                     })),
                   },
                   poolMappingInfo: dockerProviderSettings.poolMappingInfo,
-                  userData: dockerProviderSettings.userData,
                   mergeUserData: dockerProviderSettings.mergeUserData,
+                  userData: dockerProviderSettings.userData,
                   securityGroups: dockerProviderSettings.securityGroups,
                 },
               },
@@ -161,6 +162,9 @@ export const getFormSchema = ({
     staticProviderSettings: {
       "ui:data-cy": "static-provider-settings",
       "ui:ObjectFieldTemplate": CardFieldTemplate,
+      mergeUserData: {
+        "ui:elementWrapperCSS": mergeCheckboxCSS,
+      },
       userData: {
         "ui:widget": "textarea",
         "ui:elementWrapperCSS": textAreaCSS,
@@ -173,6 +177,9 @@ export const getFormSchema = ({
     dockerProviderSettings: {
       "ui:data-cy": "docker-provider-settings",
       "ui:ObjectFieldTemplate": CardFieldTemplate,
+      mergeUserData: {
+        "ui:elementWrapperCSS": mergeCheckboxCSS,
+      },
       userData: {
         "ui:widget": "textarea",
         "ui:elementWrapperCSS": textAreaCSS,
@@ -204,12 +211,15 @@ export const getFormSchema = ({
     },
     ec2FleetProviderSettings: {
       "ui:data-cy": "ec2-fleet-provider-settings",
-      "ui:addable": awsRegions.length !== fleetRegions.length,
+      "ui:addable": awsRegions.length !== configuredRegions.length,
       "ui:addButtonText": "Add region settings",
       "ui:orderable": false,
       "ui:useExpandableCard": true,
       items: {
         "ui:displayTitle": "New AWS Region",
+        mergeUserData: {
+          "ui:elementWrapperCSS": mergeCheckboxCSS,
+        },
         userData: {
           "ui:widget": "textarea",
           "ui:elementWrapperCSS": textAreaCSS,
@@ -221,7 +231,7 @@ export const getFormSchema = ({
         region: {
           "ui:data-cy": "region-select",
           "ui:allowDeselect": false,
-          "ui:enumDisabled": fleetRegions,
+          "ui:enumDisabled": configuredRegions,
         },
         amiId: {
           "ui:placeholder": "e.g. ami-1ecba176",
@@ -236,6 +246,10 @@ export const getFormSchema = ({
           },
           useCapacityOptimization: {
             "ui:data-cy": "use-capacity-optimization",
+            "ui:bold": true,
+            "ui:description":
+              "Use the capacity-optimized allocation strategy for spot (default: lowest-cost)",
+            "ui:elementWrapperCSS": capacityCheckboxCSS,
           },
         },
         vpcOptions: {
@@ -244,16 +258,18 @@ export const getFormSchema = ({
           },
           subnetId: {
             "ui:placeholder": "e.g. subnet-xxxx",
+            "ui:elementWrapperCSS": indentCSS,
           },
           subnetPrefix: {
             "ui:description":
-              "Will look for subnets like <prefix>.subnet_1a, <prefix>.subnet_1b, etc.",
+              "Looks for subnets like <prefix>.subnet_1a, <prefix>.subnet_1b, etc.",
+            "ui:elementWrapperCSS": indentCSS,
           },
         },
         mountPoints: {
           "ui:data-cy": "mount-points",
           "ui:addButtonText": "Add mount point",
-          "ui:orderable": true,
+          "ui:orderable": false,
           "ui:topAlignDelete": true,
           items: {
             "ui:ObjectFieldTemplate": AccordionFieldTemplate,
@@ -268,6 +284,25 @@ export const getFormSchema = ({
 const textAreaCSS = css`
   box-sizing: border-box;
   max-width: ${STANDARD_FIELD_WIDTH}px;
+  textarea {
+    min-height: 140px;
+  }
+`;
+
+const mergeCheckboxCSS = css`
+  max-width: ${STANDARD_FIELD_WIDTH}px;
+  display: flex;
+  justify-content: flex-end;
+  margin-bottom: -20px;
+`;
+
+const capacityCheckboxCSS = css`
+  max-width: ${STANDARD_FIELD_WIDTH}px;
+`;
+
+const indentCSS = css`
+  box-sizing: border-box;
+  padding-left: ${size.m};
 `;
 
 const poolMappingInfoCss = css`

diff --git a/src/pages/distroSettings/tabs/ProviderTab/schemaFields.ts b/src/pages/distroSettings/tabs/ProviderTab/schemaFields.ts
@@ -102,8 +102,7 @@ const fleetInstanceType = {
 
 const useCapacityOptimization = {
   type: "boolean" as "boolean",
-  title:
-    "Use the capacity-optimized allocation strategy for spot (default: lowest-cost)",
+  title: "Capacity optimization",
   default: false,
 };
 
@@ -204,6 +203,8 @@ const mountPoints = {
       deviceName: {
         type: "string" as "string",
         title: "Device Name",
+        default: "",
+        minLength: 1,
       },
       virtualName: {
         type: "string" as "string",
@@ -230,8 +231,8 @@ const mountPoints = {
 };
 
 export const staticProviderSettings = {
-  userData,
   mergeUserData,
+  userData,
   securityGroups,
 };
 
@@ -241,8 +242,8 @@ export const dockerProviderSettings = {
   registryUsername,
   registryPassword,
   poolMappingInfo,
-  userData,
   mergeUserData,
+  userData,
   securityGroups,
 };
 
@@ -252,8 +253,8 @@ export const ec2FleetProviderSettings = {
   sshKeyName,
   fleetOptions,
   instanceProfileARN,
-  userData,
   mergeUserData,
+  userData,
   securityGroups,
   vpcOptions,
   mountPoints,