Review feedback

dannykopping · dannykopping · commit 69b26cb8b7d4 · 2024-04-17T12:12:22.000+02:00
Signed-off-by: Danny Kopping &lt;danny@coder.com&gt;
diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx
@@ -13,7 +13,22 @@ const GeneralSettingsPage: FC = () => {
   const deploymentDAUsQuery = useQuery(deploymentDAUs());
   const entitlementsQuery = useQuery(entitlements());
   const enabledExperimentsQuery = useQuery(experiments());
-  const availableExperimentsQuery = useQuery(availableExperiments());
+  const safeExperimentsQuery = useQuery(availableExperiments());
+
+  const safeExperiments: string[] = [];
+  const invalidExperiments: string[] = [];
+
+  (enabledExperimentsQuery.data ?? []).forEach(function (exp) {
+    const found = (safeExperimentsQuery.data?.safe ?? []).find((value) => {
+      return exp === value;
+    });
+
+    if (found) {
+      safeExperiments.push(exp);
+    } else {
+      invalidExperiments.push(exp);
+    }
+  });
 
   return (
     <>
@@ -25,8 +40,8 @@ const GeneralSettingsPage: FC = () => {
         deploymentDAUs={deploymentDAUsQuery.data}
         deploymentDAUsError={deploymentDAUsQuery.error}
         entitlements={entitlementsQuery.data}
-        enabledExperiments={enabledExperimentsQuery.data ?? []}
-        safeExperiments={availableExperimentsQuery.data?.safe ?? []}
+        invalidExperiments={invalidExperiments}
+        safeExperiments={safeExperiments}
       />
     </>
   );
diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx
@@ -40,7 +40,7 @@ const meta: Meta<typeof GeneralSettingsPageView> = {
       },
     ],
     deploymentDAUs: MockDeploymentDAUResponse,
-    enabledExperiments: [],
+    invalidExperiments: [],
     safeExperiments: [],
   },
 };
@@ -104,7 +104,7 @@ export const allExperimentsEnabled: Story = {
       },
     ],
     safeExperiments: ["shared-ports"],
-    enabledExperiments: ["shared-ports"],
+    invalidExperiments: ["invalid"],
   },
 };
 
@@ -140,6 +140,6 @@ export const invalidExperimentsEnabled: Story = {
       },
     ],
     safeExperiments: ["shared-ports"],
-    enabledExperiments: ["invalid"],
+    invalidExperiments: ["invalid"],
   },
 };
diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx
@@ -24,37 +24,18 @@ export type GeneralSettingsPageViewProps = {
   deploymentDAUs?: DAUsResponse;
   deploymentDAUsError: unknown;
   entitlements: Entitlements | undefined;
-  enabledExperiments: Experiments | string[];
-  safeExperiments: Experiments | string[];
+  readonly invalidExperiments: Experiments | string[];
+  readonly safeExperiments: Experiments | string[];
 };
 
 export const GeneralSettingsPageView: FC<GeneralSettingsPageViewProps> = ({
   deploymentOptions,
   deploymentDAUs,
   deploymentDAUsError,
   entitlements,
-  enabledExperiments,
   safeExperiments,
+  invalidExperiments,
 }) => {
-  const safe: string[] = [];
-  const invalid: string[] = [];
-
-  enabledExperiments.forEach(function (exp) {
-    let found = false;
-    safeExperiments.forEach(function (name) {
-      if (exp === name) {
-        found = true;
-        return;
-      }
-    });
-
-    if (found) {
-      safe.push(exp);
-    } else {
-      invalid.push(exp);
-    }
-  });
-
   return (
     <>
       <Header
@@ -81,11 +62,11 @@ export const GeneralSettingsPageView: FC<GeneralSettingsPageViewProps> = ({
             </ChartSection>
           </div>
         )}
-        {invalid.length > 0 && (
+        {invalidExperiments.length > 0 && (
           <Alert severity="warning">
             <AlertTitle>Invalid experiments in use:</AlertTitle>
             <ul>
-              {invalid.map((it) => (
+              {invalidExperiments.map((it) => (
                 <li key={it}>
                   <pre>{it}</pre>
                 </li>
@@ -110,7 +91,7 @@ export const GeneralSettingsPageView: FC<GeneralSettingsPageViewProps> = ({
             "Wildcard Access URL",
             "Experiments",
           )}
-          additionalValues={safe}
+          additionalValues={safeExperiments}
         />
       </Stack>
     </>
diff --git a/site/src/pages/DeploySettingsPage/Option.tsx b/site/src/pages/DeploySettingsPage/Option.tsx
@@ -1,5 +1,5 @@
 import { css, type Interpolation, type Theme, useTheme } from "@emotion/react";
-import LabelImportant from "@mui/icons-material/LabelImportant";
+import BuildCircleOutlinedIcon from "@mui/icons-material/BuildCircleOutlined";
 import type { FC, HTMLAttributes, PropsWithChildren } from "react";
 import { DisabledBadge, EnabledBadge } from "components/Badges/Badges";
 import { MONOSPACE_FONT_FAMILY } from "theme/constants";
@@ -91,11 +91,11 @@ export const OptionValue: FC<OptionValueProps> = (props) => {
                 }}
               >
                 {isEnabled && (
-                  <LabelImportant
+                  <BuildCircleOutlinedIcon
                     css={(theme) => ({
                       width: 16,
                       height: 16,
-                      color: theme.palette.info.light,
+                      color: theme.palette.mode,
                       margin: "0 8px",
                     })}
                   />
diff --git a/site/src/pages/DeploySettingsPage/optionValue.ts b/site/src/pages/DeploySettingsPage/optionValue.ts
@@ -38,13 +38,13 @@ export function optionValue(
         ([key, value]) => `"${key}"->"${value}"`,
       );
     case "Experiments": {
-      const experimentMap: Record<string, boolean> | undefined =
-        additionalValues?.reduce(
-          (acc, v) => {
-            return { ...acc, [v]: option.value.includes("*") };
-          },
-          {} as Record<string, boolean>,
-        );
+      const experimentMap = additionalValues?.reduce<Record<string, boolean>>(
+        (acc, v) => {
+          acc[v] = option.value.includes("*");
+          return acc;
+        },
+        {},
+      );
 
       if (!experimentMap) {
         break;
