Fix unreleased UI bug in OS settings modal (#14434)

frontend/interfaces/mdm.ts

@@ -98,7 +98,15 @@ export type IWindowsDiskEncryptionStatus = Extract<
 export const isWindowsDiskEncryptionStatus = (
   status: DiskEncryptionStatus
 ): status is IWindowsDiskEncryptionStatus => {
-  return !["action_required", "removing_enforcement"].includes(status);
+  switch (status) {
+    case "verified":
+    case "verifying":
+    case "enforcing":
+    case "failed":
+      return true;
+    default:
+      return false;
+  }
 };
 
 export const FLEET_FILEVAULT_PROFILE_DISPLAY_NAME = "Disk encryption";

frontend/pages/hosts/details/MacSettingsModal/MacSettingsModal.tsx

@@ -19,13 +19,15 @@ const MacSettingsModal = ({
   hostMDMData,
   onClose,
 }: IMacSettingsModalProps) => {
+  // the caller should ensure that hostMDMData is not undefined and that platform is "windows" or
+  // "darwin", otherwise we will allow an empty modal will be rendered.
+  // https://fleetdm.com/handbook/company/why-this-way#why-make-it-obvious-when-stuff-breaks
+
   const memoizedTableData = useMemo(
     () => generateTableData(hostMDMData, platform),
     [hostMDMData, platform]
   );
 
-  if (!platform) return null;
-
   return (
     <Modal
       title="OS settings"
@@ -34,7 +36,7 @@ const MacSettingsModal = ({
       width="large"
     >
       <>
-        <MacSettingsTable tableData={memoizedTableData} />
+        <MacSettingsTable tableData={memoizedTableData || []} />
         <div className="modal-cta-wrap">
           <Button variant="brand" onClick={onClose}>
             Done

frontend/pages/hosts/details/MacSettingsModal/MacSettingsTable/MacSettingStatusCell/MacSettingStatusCell.tsx

@@ -12,7 +12,7 @@ import {
 
 import {
   isMdmProfileStatus,
-  MacSettingsTableStatusValue,
+  OsSettingsTableStatusValue,
 } from "../MacSettingsTableConfig";
 import TooltipContent, {
   TooltipInnerContentFunc,
@@ -29,7 +29,7 @@ type ProfileDisplayOption = {
 } | null;
 
 type OperationTypeOption = Record<
-  MacSettingsTableStatusValue,
+  OsSettingsTableStatusValue,
   ProfileDisplayOption
 >;
 type ProfileDisplayConfig = Record<
@@ -135,7 +135,7 @@ const WINDOWS_DISK_ENCRYPTION_DISPLAY_CONFIG: WindowsDiskEncryptionDisplayConfig
 };
 
 interface IMacSettingStatusCellProps {
-  status: MacSettingsTableStatusValue;
+  status: OsSettingsTableStatusValue;
   operationType: MacMdmProfileOperationType | null;
   profileName: string;
 }

frontend/pages/hosts/details/MacSettingsModal/MacSettingsTable/MacSettingsTable.tsx

@@ -1,12 +1,12 @@
 import React from "react";
 import TableContainer from "components/TableContainer";
 
-import tableHeaders, { IMacSettingsTableRow } from "./MacSettingsTableConfig";
+import tableHeaders, { ITableRowOsSettings } from "./MacSettingsTableConfig";
 
 const baseClass = "macsettings-table";
 
 interface IMacSettingsTableProps {
-  tableData?: IMacSettingsTableRow[];
+  tableData?: ITableRowOsSettings[];
 }
 
 const MacSettingsTable = ({ tableData }: IMacSettingsTableProps) => {

frontend/pages/hosts/details/MacSettingsModal/MacSettingsTable/MacSettingsTableConfig.tsx

@@ -14,11 +14,11 @@ import TruncatedTextCell from "components/TableContainer/DataTable/TruncatedText
 import MacSettingStatusCell from "./MacSettingStatusCell";
 import { generateWinDiskEncryptionProfile } from "../../helpers";
 
-export interface IMacSettingsTableRow extends Omit<IHostMdmProfile, "status"> {
+export interface ITableRowOsSettings extends Omit<IHostMdmProfile, "status"> {
-  status: MacSettingsTableStatusValue;
+  status: OsSettingsTableStatusValue;
 }
 
-export type MacSettingsTableStatusValue = MdmProfileStatus | "action_required";
+export type OsSettingsTableStatusValue = MdmProfileStatus | "action_required";
 
 export const isMdmProfileStatus = (
   status: string
@@ -38,7 +38,7 @@ interface ICellProps {
     value: string;
   };
   row: {
-    original: IMacSettingsTableRow;
+    original: ITableRowOsSettings;
   };
 }
 
@@ -98,42 +98,32 @@ const tableHeaders: IDataColumn[] = [
   },
 ];
 
-export const generateTableData = (
+const makeWindowsRows = ({ os_settings }: IHostMdmData) => {
-  hostMDMData?: IHostMdmData,
-  platform?: string
-) => {
-  if (!platform) return [];
-
-  let rows: IMacSettingsTableRow[] = [];
-  if (!hostMDMData) {
-    return rows;
-  }
-
   if (
-    platform === "windows" &&
+    !os_settings?.disk_encryption?.status ||
-    hostMDMData.os_settings?.disk_encryption.status &&
+    !isWindowsDiskEncryptionStatus(os_settings.disk_encryption.status)
-    isWindowsDiskEncryptionStatus(
-      hostMDMData.os_settings.disk_encryption.status
-    )
   ) {
-    rows.push(
+    return null;
-      generateWinDiskEncryptionProfile(
-        hostMDMData.os_settings.disk_encryption.status
-      )
-    );
-    return rows;
   }
 
-  const { profiles, macos_settings } = hostMDMData;
+  const rows: ITableRowOsSettings[] = [];
+  rows.push(
+    generateWinDiskEncryptionProfile(os_settings.disk_encryption.status)
+  );
 
+  return rows;
+};
+
+const makeDarwinRows = ({
+  profiles,
+  macos_settings,
+}: IHostMdmData): ITableRowOsSettings[] | null => {
   if (!profiles) {
-    return rows;
+    return null;
   }
 
-  if (
+  let rows: ITableRowOsSettings[] = profiles;
-    platform === "darwin" &&
+  if (macos_settings?.disk_encryption === "action_required") {
-    macos_settings?.disk_encryption === "action_required"
-  ) {
     rows = profiles.map((p) => {
       // TODO: this is a brittle check for the filevault profile
       // it would be better to match on the identifier but it is not
@@ -148,4 +138,22 @@ export const generateTableData = (
   return rows;
 };
 
+export const generateTableData = (
+  hostMDMData?: IHostMdmData,
+  platform?: string
+) => {
+  if (!platform || !hostMDMData) {
+    return null;
+  }
+
+  switch (platform) {
+    case "windows":
+      return makeWindowsRows(hostMDMData);
+    case "darwin":
+      return makeDarwinRows(hostMDMData);
+    default:
+      return null;
+  }
+};
+
 export default tableHeaders;

frontend/pages/hosts/details/cards/HostSummary/MacSettingsIndicator/MacSettingsIndicator.tsx

@@ -1,7 +1,7 @@
 import React from "react";
 import ReactTooltip from "react-tooltip";
 
-import { IHostMdmProfile } from "interfaces/mdm";
+import { IHostMdmProfile, MdmProfileStatus } from "interfaces/mdm";
 
 import Icon from "components/Icon";
 import Button from "components/buttons/Button";
@@ -9,7 +9,11 @@ import { IconNames } from "components/icons";
 
 const baseClass = "mac-settings-indicator";
 
-type MacProfileStatus = "Failed" | "Verifying" | "Pending" | "Verified";
+type MdmProfileStatusForDisplay =
+  | "Failed"
+  | "Pending"
+  | "Verifying"
+  | "Verified";
 
 interface IStatusDisplayOption {
   iconName: Extract<
@@ -18,7 +22,10 @@ interface IStatusDisplayOption {
   >;
   tooltipText: string;
 }
-type StatusDisplayOptions = Record<MacProfileStatus, IStatusDisplayOption>;
+type StatusDisplayOptions = Record<
+  MdmProfileStatusForDisplay,
+  IStatusDisplayOption
+>;
 
 const STATUS_DISPLAY_OPTIONS: StatusDisplayOptions = {
   Verified: {
@@ -44,27 +51,59 @@ const STATUS_DISPLAY_OPTIONS: StatusDisplayOptions = {
   },
 };
 
+const countHostProfilesByStatus = (
+  hostSettings: IHostMdmProfile[]
+): Record<MdmProfileStatus, number> => {
+  return hostSettings.reduce(
+    (acc, { status }) => {
+      if (status === "failed") {
+        acc.failed += 1;
+      } else if (status === "pending") {
+        acc.pending += 1;
+      } else if (status === "verifying") {
+        acc.verifying += 1;
+      } else if (status === "verified") {
+        acc.verified += 1;
+      }
+
+      return acc;
+    },
+    {
+      failed: 0,
+      pending: 0,
+      verifying: 0,
+      verified: 0,
+    }
+  );
+};
+
 /**
  * Returns the displayed status of the macOS settings field based on the
  * profile statuses.
  * If any profile has a status of "failed", the status will be displayed as "Failed" and
  * continues to fall through to "Pending" and "Verifying" if any profiles have those statuses.
- * Finally if all profiles have a status of "verified", the status will be displayed as "Verified".
+ * If all profiles have a status of "verified", the status will be displayed as "Verified".
+ *
+ * The default status will be displayed as "Failed".
+ * https://fleetdm.com/handbook/company/why-this-way#why-make-it-obvious-when-stuff-breaks
  */
-const getMacProfileStatus = (
+const getHostProfilesStatusForDisplay = (
   hostMacSettings: IHostMdmProfile[]
-): MacProfileStatus => {
+): MdmProfileStatusForDisplay => {
-  const statuses = hostMacSettings.map((setting) => setting.status);
+  const counts = countHostProfilesByStatus(hostMacSettings);
-  if (statuses.includes("failed")) {
+  switch (true) {
-    return "Failed";
+    case !!counts.failed:
+      return "Failed";
+    case !!counts.pending:
+      return "Pending";
+    case !!counts.verifying:
+      return "Verifying";
+    case counts.verified === hostMacSettings.length:
+      return "Verified";
+    default:
+      // something is broken
+      return "Failed";
   }
-  if (statuses.includes("pending")) {
-    return "Pending";
-  }
-  if (statuses.includes("verifying")) {
-    return "Verifying";
-  }
-  return "Verified";
 };
 
 interface IMacSettingsIndicatorProps {
@@ -75,9 +114,16 @@ const MacSettingsIndicator = ({
   profiles,
   onClick,
 }: IMacSettingsIndicatorProps): JSX.Element => {
-  const macProfileStatus = getMacProfileStatus(profiles);
+  if (!profiles.length) {
+    // the caller should ensure that this never happens, but just in case we return a default
+    // to make it more obvious that something is wrong.
+    // https://fleetdm.com/handbook/company/why-this-way#why-make-it-obvious-when-stuff-breaks
+    return <span className={`${baseClass} info-flex__data`}>Unavailable</span>;
+  }
 
-  const statusDisplayOption = STATUS_DISPLAY_OPTIONS[macProfileStatus];
+  const displayStatus = getHostProfilesStatusForDisplay(profiles);
+
+  const statusDisplayOption = STATUS_DISPLAY_OPTIONS[displayStatus];
 
   return (
     <span className={`${baseClass} info-flex__data`}>
@@ -93,7 +139,7 @@ const MacSettingsIndicator = ({
           variant="text-link"
           className={`${baseClass}__button`}
         >
-          {macProfileStatus}
+          {displayStatus}
         </Button>
       </span>
       <ReactTooltip