Code review updates

fleetdm · Jul 3, 2024 · 393681c · 393681c
1 parent 96af97f
commit 393681c
Show file tree

Hide file tree

Showing 23 changed files with 67 additions and 87 deletions.
diff --git a/frontend/components/PlatformCompatibility/PlatformCompatibility.tests.tsx b/frontend/components/PlatformCompatibility/PlatformCompatibility.tests.tsx
@@ -7,7 +7,7 @@ describe("Platform compatibility", () => {
   it("renders compatible platforms", () => {
     render(
       <PlatformCompatibility
-        compatiblePlatforms={["macOS", "Windows"]}
+        compatiblePlatforms={["darwin", "windows"]}
         error={null}
       />
     );
@@ -38,7 +38,7 @@ describe("Platform compatibility", () => {
   it("renders error state", () => {
     render(
       <PlatformCompatibility
-        compatiblePlatforms={["macOS"]}
+        compatiblePlatforms={["macos"]}
         error={{ name: "Error", message: "The resource was not found." }}
       />
     );

diff --git a/frontend/components/PlatformCompatibility/PlatformCompatibility.tsx b/frontend/components/PlatformCompatibility/PlatformCompatibility.tsx
@@ -2,8 +2,8 @@ import React from "react";
 
 import {
   DisplayPlatform,
-  SupportedDisplayPlatform,
-  SupportedPlatform,
+  QueryableDisplayPlatform,
+  QueryablePlatform,
 } from "interfaces/platform";
 import { PLATFORM_DISPLAY_NAMES } from "utilities/constants";
 
@@ -22,12 +22,12 @@ const DISPLAY_ORDER = [
   "Windows",
   "Linux",
   "ChromeOS",
-] as SupportedDisplayPlatform[];
+] as QueryableDisplayPlatform[];
 
 const ERROR_NO_COMPATIBLE_TABLES = Error("no tables in query");
 
 const formatPlatformsForDisplay = (
-  compatiblePlatforms: SupportedPlatform[]
+  compatiblePlatforms: QueryablePlatform[]
 ): DisplayPlatform[] => {
   return compatiblePlatforms.map((str) => PLATFORM_DISPLAY_NAMES[str] || str);
 };
@@ -56,8 +56,6 @@ const PlatformCompatibility = ({
     return null;
   }
 
-  console.log("compatiblePlat", compatiblePlatforms);
-
   const displayPlatforms = formatPlatformsForDisplay(compatiblePlatforms);
 
   const renderCompatiblePlatforms = () => {

diff --git a/frontend/components/TableContainer/DataTable/PlatformCell/PlatformCell.tests.tsx b/frontend/components/TableContainer/DataTable/PlatformCell/PlatformCell.tests.tsx
@@ -2,10 +2,10 @@ import React from "react";
 import { render, screen } from "@testing-library/react";
 import { DEFAULT_EMPTY_CELL_VALUE } from "utilities/constants";
 
-import { SupportedPlatform } from "interfaces/platform";
+import { QueryablePlatform } from "interfaces/platform";
 import PlatformCell from "./PlatformCell";
 
-const PLATFORMS: SupportedPlatform[] = ["windows", "darwin", "linux", "chrome"];
+const PLATFORMS: QueryablePlatform[] = ["windows", "darwin", "linux", "chrome"];
 
 describe("Platform cell", () => {
   it("renders platform icons in correct order", () => {

diff --git a/frontend/components/TableContainer/DataTable/PlatformCell/PlatformCell.tsx b/frontend/components/TableContainer/DataTable/PlatformCell/PlatformCell.tsx
@@ -1,22 +1,22 @@
 import React from "react";
 import Icon from "components/Icon";
-import { SupportedPlatform } from "interfaces/platform";
+import { QueryablePlatform } from "interfaces/platform";
 import { DEFAULT_EMPTY_CELL_VALUE } from "utilities/constants";
 
 interface IPlatformCellProps {
-  platforms: SupportedPlatform[];
+  platforms: QueryablePlatform[];
 }
 
 const baseClass = "platform-cell";
 
-const ICONS: Record<string, SupportedPlatform> = {
+const ICONS: Record<string, QueryablePlatform> = {
   darwin: "darwin",
   windows: "windows",
   linux: "linux",
   chrome: "chrome",
 };
 
-const DISPLAY_ORDER: SupportedPlatform[] = [
+const DISPLAY_ORDER: QueryablePlatform[] = [
   "darwin",
   "windows",
   "linux",

diff --git a/frontend/hooks/usePlatformCompatibility.tsx b/frontend/hooks/usePlatformCompatibility.tsx
@@ -1,13 +1,13 @@
 import React, { useCallback, useState } from "react";
 import { useDebouncedCallback } from "use-debounce";
 
-import { SupportedPlatform, SUPPORTED_PLATFORMS } from "interfaces/platform";
+import { QueryablePlatform, SUPPORTED_PLATFORMS } from "interfaces/platform";
 import { checkPlatformCompatibility } from "utilities/sql_tools";
 
 import PlatformCompatibility from "components/PlatformCompatibility";
 
 export interface IPlatformCompatibility {
-  getCompatiblePlatforms: () => SupportedPlatform[];
+  getCompatiblePlatforms: () => QueryablePlatform[];
   setCompatiblePlatforms: (sqlString: string) => void;
   render: () => JSX.Element;
 }
@@ -16,7 +16,7 @@ const DEBOUNCE_DELAY = 300;
 
 const usePlatformCompatibility = (): IPlatformCompatibility => {
   const [compatiblePlatforms, setCompatiblePlatforms] = useState<
-    SupportedPlatform[] | null
+    QueryablePlatform[] | null
   >(null);
   const [error, setError] = useState<Error | null>(null);
 

diff --git a/frontend/hooks/usePlatformSelector.tsx b/frontend/hooks/usePlatformSelector.tsx
@@ -4,14 +4,14 @@ import { forEach } from "lodash";
 import {
   SelectedPlatformString,
   SUPPORTED_PLATFORMS,
-  SupportedPlatform,
+  QueryablePlatform,
 } from "interfaces/platform";
 
 import PlatformSelector from "components/PlatformSelector";
 
 export interface IPlatformSelector {
   setSelectedPlatforms: (platforms: string[]) => void;
-  getSelectedPlatforms: () => SupportedPlatform[];
+  getSelectedPlatforms: () => QueryablePlatform[];
   isAnyPlatformSelected: boolean;
   render: () => JSX.Element;
   disabled?: boolean;

diff --git a/frontend/interfaces/osquery_table.ts b/frontend/interfaces/osquery_table.ts
@@ -1,5 +1,5 @@
 import PropTypes from "prop-types";
-import { SupportedPlatform, SupportedDisplayPlatform } from "./platform";
+import { QueryablePlatform, QueryableDisplayPlatform } from "./platform";
 
 export default PropTypes.shape({
   columns: PropTypes.arrayOf(
@@ -24,7 +24,7 @@ export type ColumnType =
   | "string"; // TODO: Why do we have type string, STRING, and text in schema.json?
 
 // TODO: Replace with one or the other once osquery_fleet_schema.json follows one type or other
-export type TableSchemaPlatform = SupportedDisplayPlatform | SupportedPlatform;
+export type TableSchemaPlatform = QueryableDisplayPlatform | QueryablePlatform;
 export interface IQueryTableColumn {
   name: string;
   description: string;

diff --git a/frontend/interfaces/platform.ts b/frontend/interfaces/platform.ts
@@ -14,31 +14,31 @@ export type Platform =
   | "ios"
   | "ipados";
 
-export type SupportedDisplayPlatform = Exclude<
+export type QueryableDisplayPlatform = Exclude<
   DisplayPlatform,
   "iOS" | "iPadOS"
 >;
 
-export type SupportedPlatform = Exclude<Platform, "ios" | "ipados">;
+export type QueryablePlatform = Exclude<Platform, "ios" | "ipados">;
 
-export const SUPPORTED_PLATFORMS: SupportedPlatform[] = [
+export const SUPPORTED_PLATFORMS: QueryablePlatform[] = [
   "darwin",
   "windows",
   "linux",
   "chrome",
 ];
 
-export type SelectedPlatform = SupportedPlatform | "all";
+export type SelectedPlatform = QueryablePlatform | "all";
 
 export type SelectedPlatformString =
   | ""
-  | SupportedPlatform
-  | `${SupportedPlatform},${SupportedPlatform}`
-  | `${SupportedPlatform},${SupportedPlatform},${SupportedPlatform}`
-  | `${SupportedPlatform},${SupportedPlatform},${SupportedPlatform},${SupportedPlatform}`;
+  | QueryablePlatform
+  | `${QueryablePlatform},${QueryablePlatform}`
+  | `${QueryablePlatform},${QueryablePlatform},${QueryablePlatform}`
+  | `${QueryablePlatform},${QueryablePlatform},${QueryablePlatform},${QueryablePlatform}`;
 
 // TODO: revisit this approach pending resolution of https://github.com/fleetdm/fleet/issues/3555.
-export const MACADMINS_EXTENSION_TABLES: Record<string, SupportedPlatform[]> = {
+export const MACADMINS_EXTENSION_TABLES: Record<string, QueryablePlatform[]> = {
   file_lines: ["darwin", "linux", "windows"],
   filevault_users: ["darwin"],
   google_chrome_profiles: ["darwin", "linux", "windows"],

diff --git a/frontend/interfaces/schedulable_query.ts b/frontend/interfaces/schedulable_query.ts
@@ -3,7 +3,7 @@ import PropTypes from "prop-types";
 
 import { IFormField } from "./form_field";
 import { IPack } from "./pack";
-import { SelectedPlatformString, SupportedPlatform } from "./platform";
+import { SelectedPlatformString, QueryablePlatform } from "./platform";
 
 // Query itself
 export interface ISchedulableQuery {
@@ -32,7 +32,7 @@ export interface ISchedulableQuery {
 
 export interface IEnhancedQuery extends ISchedulableQuery {
   performance: string;
-  platforms: SupportedPlatform[];
+  platforms: QueryablePlatform[];
 }
 export interface ISchedulableQueryStats {
   user_time_p50?: number | null;

diff --git a/frontend/pages/DashboardPage/cards/HostsSummary/HostsSummary.tsx b/frontend/pages/DashboardPage/cards/HostsSummary/HostsSummary.tsx
@@ -1,4 +1,4 @@
-import React from "react";
+import React, { useCallback } from "react";
 import PATHS from "router/paths";
 
 import { PLATFORM_NAME_TO_LABEL_NAME } from "pages/DashboardPage/helpers";
@@ -45,10 +45,16 @@ const HostsSummary = ({
     opacity = isLoadingHostsSummary ? { opacity: 0.4 } : { opacity: 1 };
   }
 
+  const getBuiltinLabelId = useCallback(
+    (platformName: keyof typeof PLATFORM_NAME_TO_LABEL_NAME) =>
+      builtInLabels?.find(
+        (builtin) => builtin.name === PLATFORM_NAME_TO_LABEL_NAME[platformName]
+      )?.id,
+    [builtInLabels]
+  );
+
   const renderMacCount = (teamId?: number) => {
-    const macLabelId = builtInLabels?.find((builtin) => {
-      return builtin.name === PLATFORM_NAME_TO_LABEL_NAME.darwin;
-    })?.id;
+    const macLabelId = getBuiltinLabelId("darwin");
 
     if (isLoadingHostsSummary || macLabelId === undefined) {
       return <></>;
@@ -69,9 +75,7 @@ const HostsSummary = ({
   };
 
   const renderWindowsCount = (teamId?: number) => {
-    const windowsLabelId = builtInLabels?.find(
-      (builtin) => builtin.name === PLATFORM_NAME_TO_LABEL_NAME.windows
-    )?.id;
+    const windowsLabelId = getBuiltinLabelId("windows");
 
     if (isLoadingHostsSummary || windowsLabelId === undefined) {
       return <></>;
@@ -91,9 +95,7 @@ const HostsSummary = ({
   };
 
   const renderLinuxCount = (teamId?: number) => {
-    const linuxLabelId = builtInLabels?.find(
-      (builtin) => builtin.name === PLATFORM_NAME_TO_LABEL_NAME.linux
-    )?.id;
+    const linuxLabelId = getBuiltinLabelId("linux");
 
     if (isLoadingHostsSummary || linuxLabelId === undefined) {
       return <></>;
@@ -113,9 +115,7 @@ const HostsSummary = ({
   };
 
   const renderChromeCount = (teamId?: number) => {
-    const chromeLabelId = builtInLabels?.find(
-      (builtin) => builtin.name === PLATFORM_NAME_TO_LABEL_NAME.chrome
-    )?.id;
+    const chromeLabelId = getBuiltinLabelId("chrome");
 
     if (isLoadingHostsSummary || chromeLabelId === undefined) {
       return <></>;
@@ -136,9 +136,7 @@ const HostsSummary = ({
   };
 
   const renderIosCount = (teamId?: number) => {
-    const iosLabelId = builtInLabels?.find(
-      (builtin) => builtin.name === PLATFORM_NAME_TO_LABEL_NAME.ios
-    )?.id;
+    const iosLabelId = getBuiltinLabelId("ios");
 
     if (isLoadingHostsSummary || iosLabelId === undefined) {
       return <></>;
@@ -159,9 +157,7 @@ const HostsSummary = ({
   };
 
   const renderIpadosCount = (teamId?: number) => {
-    const ipadosLabelId = builtInLabels?.find(
-      (builtin) => builtin.name === PLATFORM_NAME_TO_LABEL_NAME.ipados
-    )?.id;
+    const ipadosLabelId = getBuiltinLabelId("ipados");
 
     if (isLoadingHostsSummary || ipadosLabelId === undefined) {
       return <></>;

diff --git a/frontend/pages/DashboardPage/cards/HostsSummary/_styles.scss b/frontend/pages/DashboardPage/cards/HostsSummary/_styles.scss
@@ -2,7 +2,6 @@
   margin-top: $pad-large;
   width: 100%;
   display: flex;
-  flex-direction: row;
   justify-content: space-around;
   flex-wrap: wrap;
   font-size: $x-small;

diff --git a/frontend/pages/DashboardPage/cards/OperatingSystems/OperatingSystems.tsx b/frontend/pages/DashboardPage/cards/OperatingSystems/OperatingSystems.tsx
@@ -5,7 +5,6 @@ import {
   OS_END_OF_LIFE_LINK_BY_PLATFORM,
   OS_VENDOR_BY_PLATFORM,
 } from "interfaces/operating_system";
-import { Platform } from "interfaces/platform";
 import {
   getOSVersions,
   IGetOSVersionsQueryKey,

diff --git a/frontend/pages/DashboardPage/helpers.tsx → frontend/pages/DashboardPage/helpers.ts b/frontend/pages/DashboardPage/helpers.tsx → frontend/pages/DashboardPage/helpers.ts
diff --git a/frontend/pages/ManageControlsPage/OSSettings/cards/CustomSettings/CustomSettings.tsx b/frontend/pages/ManageControlsPage/OSSettings/cards/CustomSettings/CustomSettings.tsx
@@ -131,9 +131,9 @@ const CustomSettings = ({
       return <Spinner />;
     }
 
-    // if (isErrorProfiles) {
-    //   return <DataError />;
-    // }
+    if (isErrorProfiles) {
+      return <DataError />;
+    }
 
     if (!profiles?.length) {
       return null;

diff --git a/...ntrolsPage/OSSettings/cards/CustomSettings/components/ProfileListItem/ProfileListItem.tsx b/...ntrolsPage/OSSettings/cards/CustomSettings/components/ProfileListItem/ProfileListItem.tsx
@@ -39,7 +39,7 @@ const ProfileDetails = ({
 }: IProfileDetailsProps) => {
   const getPlatformName = () => {
     if (platform === "windows") return "Windows";
-    return isDDM ? "macOS (declaration)" : "macOS, iOS, iPadOS"; // TODO: Confirm this is correct logic and copy text
+    return isDDM ? "macOS (declaration)" : "macOS, iOS, iPadOS";
   };
 
   return (

diff --git a/frontend/pages/ManageControlsPage/OSUpdates/components/EmptyTargetForm/EmptyTargetForm.tsx b/frontend/pages/ManageControlsPage/OSUpdates/components/EmptyTargetForm/EmptyTargetForm.tsx
@@ -1,15 +1,5 @@
-import React, { useContext, useState } from "react";
-import { isEmpty } from "lodash";
+import React from "react";
 
-import { APP_CONTEXT_NO_TEAM_ID } from "interfaces/team";
-import { NotificationContext } from "context/notification";
-import configAPI from "services/entities/config";
-import teamsAPI from "services/entities/teams";
-
-// @ts-ignore
-import InputField from "components/forms/fields/InputField";
-import Button from "components/buttons/Button";
-import validatePresence from "components/forms/validators/validate_presence";
 import CustomLink from "components/CustomLink";
 
 const baseClass = "empty-target-form";
@@ -20,7 +10,7 @@ interface IEmptyTargetFormProps {
 
 const EmptyTargetForm = ({ targetPlatform }: IEmptyTargetFormProps) => {
   return (
-    <>
+    <div className={baseClass}>
       <p>
         <b>{targetPlatform} updates are coming soon.</b>
       </p>
@@ -32,7 +22,7 @@ const EmptyTargetForm = ({ targetPlatform }: IEmptyTargetFormProps) => {
           newTab
         />
       </p>
-    </>
+    </div>
   );
 };
 

diff --git a/frontend/pages/ManageControlsPage/OSUpdates/components/PlatformTabs/PlatformTabs.tsx b/frontend/pages/ManageControlsPage/OSUpdates/components/PlatformTabs/PlatformTabs.tsx
@@ -1,4 +1,4 @@
-import React, { useState } from "react";
+import React from "react";
 import { Tab, TabList, TabPanel, Tabs } from "react-tabs";
 import TabsWrapper from "components/TabsWrapper";
 
@@ -56,16 +56,16 @@ const PlatformTabs = ({
           <TabList>
             {/* Bolding text when the tab is active causes a layout shift so
             we add a hidden pseudo element with the same text string */}
-            <Tab key={"macOS"} data-text={"macOS"}>
+            <Tab key="macOS" data-text="macOS">
               macOS
             </Tab>
-            <Tab key={"Windows"} data-text={"Windows"}>
+            <Tab key="Windows" data-text="Windows">
               Windows
             </Tab>
-            <Tab key={"iOS"} data-text={"iOS"}>
+            <Tab key="iOS" data-text="iOS">
               iOS
             </Tab>
-            <Tab key={"iPadOS"} data-text={"iPadOS"}>
+            <Tab key="iPadOS" data-text="iPadOS">
               iPadOS
             </Tab>
           </TabList>