From b3e28f152247a76fbc5ae645bf5346cf008d641d Mon Sep 17 00:00:00 2001 From: Sarah Gillespie <73313222+gillespi314@users.noreply.github.com> Date: Mon, 5 Feb 2024 15:09:51 -0600 Subject: [PATCH] Fix unreleased bugs in OS updates UI (#16604) --- .../OSUpdates/OSUpdates.tsx | 86 +++++++--- .../MacOSTargetForm/MacOSTargetForm.tsx | 19 ++- .../components/NudgePreview/NudgePreview.tsx | 3 + .../components/PlatformTabs/PlatformTabs.tsx | 19 ++- .../TargetSection/TargetSection.tsx | 156 +++++++++--------- .../WindowsTargetForm/WindowsTargetForm.tsx | 27 ++- .../components/WindowsTargetForm/_styles.scss | 10 -- 7 files changed, 185 insertions(+), 135 deletions(-) delete mode 100644 frontend/pages/ManageControlsPage/OSUpdates/components/WindowsTargetForm/_styles.scss diff --git a/frontend/pages/ManageControlsPage/OSUpdates/OSUpdates.tsx b/frontend/pages/ManageControlsPage/OSUpdates/OSUpdates.tsx index 0bf5861922..0cde55cb30 100644 --- a/frontend/pages/ManageControlsPage/OSUpdates/OSUpdates.tsx +++ b/frontend/pages/ManageControlsPage/OSUpdates/OSUpdates.tsx @@ -1,15 +1,23 @@ -import React, { useContext, useEffect, useState } from "react"; +import React, { useContext, useState } from "react"; import { InjectedRouter } from "react-router"; +import { useQuery } from "react-query"; -import { IConfig } from "interfaces/config"; import { AppContext } from "context/app"; +import { IConfig } from "interfaces/config"; +import { ITeamConfig } from "interfaces/team"; + +import configAPI from "services/entities/config"; +import teamsAPI, { ILoadTeamResponse } from "services/entities/teams"; + import PremiumFeatureMessage from "components/PremiumFeatureMessage"; +import Spinner from "components/Spinner"; import NudgePreview from "./components/NudgePreview"; import TurnOnMdmMessage from "../components/TurnOnMdmMessage/TurnOnMdmMessage"; import CurrentVersionSection from "./components/CurrentVersionSection"; import TargetSection from "./components/TargetSection"; +import { generateKey } from "./components/TargetSection/TargetSection"; export type OSUpdatesSupportedPlatform = "darwin" | "windows"; @@ -34,21 +42,39 @@ interface IOSUpdates { } const OSUpdates = ({ router, teamIdForApi }: IOSUpdates) => { - const { config, isPremiumTier } = useContext(AppContext); + const { isPremiumTier, setConfig } = useContext(AppContext); - // the default platform is mac and we later update this value when we have - // done more checks. const [ - selectedPlatform, - setSelectedPlatform, - ] = useState("darwin"); + selectedPlatformTab, + setSelectedPlatformTab, + ] = useState(null); - // we have to use useEffect here as we need to update our selected platform - // state when the app config is updated. This is usually when we get the app - // config response from the server and it is no longer `null`. - useEffect(() => { - setSelectedPlatform(getSelectedPlatform(config)); - }, [config]); + // FIXME: We're calling this endpoint twice on mount because it also gets called in App.tsx + // whenever the pathname changes. We should find a way to avoid this. + const { + data: config, + isLoading: isLoadingConfig, + isError: isErrorConfig, + refetch: refetchAppConfig, + } = useQuery(["config"], () => configAPI.loadAll(), { + refetchOnWindowFocus: false, + onSuccess: (data) => setConfig(data), // update the app context with the fetched config + }); + + const { + data: teamConfig, + isLoading: isLoadingTeam, + isError: isErrorTeamConfig, + refetch: refetchTeamConfig, + } = useQuery( + ["team-config", teamIdForApi], + () => teamsAPI.load(teamIdForApi), + { + refetchOnWindowFocus: false, + enabled: !!teamIdForApi, + select: (data) => data.team, + } + ); // Not premium shows premium message if (!isPremiumTier) { @@ -59,19 +85,28 @@ const OSUpdates = ({ router, teamIdForApi }: IOSUpdates) => { ); } + // FIXME: Are these checks still necessary? if (config === null || teamIdForApi === undefined) return null; + // FIXME: We ought to display a spinner or some disabled state whenever refetching these queries + // too because a slow network can cause a disconnect between the form data and the actual data and + // we don't want the user to be editing the form data while fresh data is being fetched. We don't + // have a specified UX for this yet. + if (isLoadingConfig || isLoadingTeam) return ; + + // FIXME: Handle error states for app config and team config (need specifications for this). + // mdm is not enabled for mac or windows. if ( - !config.mdm.enabled_and_configured && - !config.mdm.windows_enabled_and_configured + !config?.mdm.enabled_and_configured && + !config?.mdm.windows_enabled_and_configured ) { return ; } - const handleSelectPlatform = (platform: OSUpdatesSupportedPlatform) => { - setSelectedPlatform(platform); - }; + // If the user has not selected a platform yet, we default to the platform that + // is enabled and configured. + const selectedPlatform = selectedPlatformTab || getSelectedPlatform(config); return (
@@ -85,9 +120,18 @@ const OSUpdates = ({ router, teamIdForApi }: IOSUpdates) => {
diff --git a/frontend/pages/ManageControlsPage/OSUpdates/components/MacOSTargetForm/MacOSTargetForm.tsx b/frontend/pages/ManageControlsPage/OSUpdates/components/MacOSTargetForm/MacOSTargetForm.tsx index 495c15e2ed..7fe0e228a8 100644 --- a/frontend/pages/ManageControlsPage/OSUpdates/components/MacOSTargetForm/MacOSTargetForm.tsx +++ b/frontend/pages/ManageControlsPage/OSUpdates/components/MacOSTargetForm/MacOSTargetForm.tsx @@ -1,6 +1,5 @@ import React, { useContext, useState } from "react"; import { isEmpty } from "lodash"; -import classnames from "classnames"; import { APP_CONTEXT_NO_TEAM_ID } from "interfaces/team"; import { NotificationContext } from "context/notification"; @@ -11,7 +10,6 @@ import teamsAPI from "services/entities/teams"; import InputField from "components/forms/fields/InputField"; import Button from "components/buttons/Button"; import validatePresence from "components/forms/validators/validate_presence"; -import { AppContext } from "context/app"; const baseClass = "mac-os-target-form"; @@ -78,14 +76,17 @@ interface IMacOSTargetFormProps { currentTeamId: number; defaultMinOsVersion: string; defaultDeadline: string; + refetchAppConfig: () => void; + refetchTeamConfig: () => void; } const MacOSTargetForm = ({ currentTeamId, defaultMinOsVersion, defaultDeadline, + refetchAppConfig, + refetchTeamConfig, }: IMacOSTargetFormProps) => { - const { setConfig } = useContext(AppContext); const { renderFlash } = useContext(NotificationContext); const [isSaving, setIsSaving] = useState(false); @@ -96,11 +97,8 @@ const MacOSTargetForm = ({ >(); const [deadlineError, setDeadlineError] = useState(); - const updateNoTeamConfig = async (updateData: IMacMdmConfigData) => { - const updatedConfig = await configAPI.update(updateData); - setConfig(updatedConfig); - }; - + // FIXME: This behaves unexpectedly when a user switches tabs or changes the teams dropdown while the form is + // submitting because this component is unmounted. const handleSubmit = async (e: React.FormEvent) => { e.preventDefault(); const errors = validateForm({ @@ -116,12 +114,15 @@ const MacOSTargetForm = ({ const updateData = createMdmConfigData(minOsVersion, deadline); try { currentTeamId === APP_CONTEXT_NO_TEAM_ID - ? await updateNoTeamConfig(updateData) + ? await configAPI.update(updateData) : await teamsAPI.update(updateData, currentTeamId); renderFlash("success", "Successfully updated minimum version!"); } catch { renderFlash("error", "Couldn’t update. Please try again."); } finally { + currentTeamId === APP_CONTEXT_NO_TEAM_ID + ? refetchAppConfig() + : refetchTeamConfig(); setIsSaving(false); } } diff --git a/frontend/pages/ManageControlsPage/OSUpdates/components/NudgePreview/NudgePreview.tsx b/frontend/pages/ManageControlsPage/OSUpdates/components/NudgePreview/NudgePreview.tsx index 314b507b14..bb1a822bcb 100644 --- a/frontend/pages/ManageControlsPage/OSUpdates/components/NudgePreview/NudgePreview.tsx +++ b/frontend/pages/ManageControlsPage/OSUpdates/components/NudgePreview/NudgePreview.tsx @@ -65,6 +65,9 @@ interface INudgePreviewProps { } const NudgePreview = ({ platform }: INudgePreviewProps) => { + // FIXME: on slow connection the image loads after the text which looks weird and can cause a + // mismatch between the text and the image when switching between platforms. We should load the + // image first and then the text. return (
diff --git a/frontend/pages/ManageControlsPage/OSUpdates/components/PlatformTabs/PlatformTabs.tsx b/frontend/pages/ManageControlsPage/OSUpdates/components/PlatformTabs/PlatformTabs.tsx index 8a158160b2..af5432a941 100644 --- a/frontend/pages/ManageControlsPage/OSUpdates/components/PlatformTabs/PlatformTabs.tsx +++ b/frontend/pages/ManageControlsPage/OSUpdates/components/PlatformTabs/PlatformTabs.tsx @@ -14,7 +14,10 @@ interface IPlatformTabsProps { defaultMacOSDeadline: string; defaultWindowsDeadlineDays: string; defaultWindowsGracePeriodDays: string; - onSelectAccordionItem: (platform: OSUpdatesSupportedPlatform) => void; + selectedPlatform: OSUpdatesSupportedPlatform; + onSelectPlatform: (platform: OSUpdatesSupportedPlatform) => void; + refetchAppConfig: () => void; + refetchTeamConfig: () => void; } const PlatformTabs = ({ @@ -23,14 +26,20 @@ const PlatformTabs = ({ defaultMacOSVersion, defaultWindowsDeadlineDays, defaultWindowsGracePeriodDays, - onSelectAccordionItem, + selectedPlatform, + onSelectPlatform, + refetchAppConfig, + refetchTeamConfig, }: IPlatformTabsProps) => { + // FIXME: This behaves unexpectedly when a user switches tabs or changes the teams dropdown while a form is + // submitting. return (
- onSelectAccordionItem(currentIndex === 0 ? "darwin" : "windows") + onSelectPlatform(currentIndex === 0 ? "darwin" : "windows") } > @@ -43,6 +52,8 @@ const PlatformTabs = ({ defaultMinOsVersion={defaultMacOSVersion} defaultDeadline={defaultMacOSDeadline} key={currentTeamId} + refetchAppConfig={refetchAppConfig} + refetchTeamConfig={refetchTeamConfig} /> @@ -51,6 +62,8 @@ const PlatformTabs = ({ defaultDeadlineDays={defaultWindowsDeadlineDays} defaultGracePeriodDays={defaultWindowsGracePeriodDays} key={currentTeamId} + refetchAppConfig={refetchAppConfig} + refetchTeamConfig={refetchTeamConfig} /> diff --git a/frontend/pages/ManageControlsPage/OSUpdates/components/TargetSection/TargetSection.tsx b/frontend/pages/ManageControlsPage/OSUpdates/components/TargetSection/TargetSection.tsx index abde65356e..3947d8f4a2 100644 --- a/frontend/pages/ManageControlsPage/OSUpdates/components/TargetSection/TargetSection.tsx +++ b/frontend/pages/ManageControlsPage/OSUpdates/components/TargetSection/TargetSection.tsx @@ -1,16 +1,8 @@ -import React, { useContext } from "react"; -import { useQuery } from "react-query"; +import React from "react"; -import { - API_NO_TEAM_ID, - APP_CONTEXT_NO_TEAM_ID, - ITeamConfig, -} from "interfaces/team"; +import { API_NO_TEAM_ID, ITeamConfig } from "interfaces/team"; import { IConfig } from "interfaces/config"; -import { AppContext } from "context/app"; -import teamsAPI, { ILoadTeamResponse } from "services/entities/teams"; -import Spinner from "components/Spinner"; import SectionHeader from "components/SectionHeader"; import MacOSTargetForm from "../MacOSTargetForm"; @@ -20,99 +12,104 @@ import { OSUpdatesSupportedPlatform } from "../../OSUpdates"; const baseClass = "os-updates-target-section"; -const getDefaultMacOSVersion = ( - currentTeam: number, - appConfig: IConfig, - teamConfig?: ITeamConfig -) => { - return currentTeam === API_NO_TEAM_ID +type GetDefaultFnParams = { + currentTeamId: number; + appConfig: IConfig; + teamConfig?: ITeamConfig; +}; + +const getDefaultMacOSVersion = ({ + currentTeamId, + appConfig, + teamConfig, +}: GetDefaultFnParams) => { + return currentTeamId === API_NO_TEAM_ID ? appConfig?.mdm.macos_updates.minimum_version ?? "" : teamConfig?.mdm?.macos_updates.minimum_version ?? ""; }; -const getDefaultMacOSDeadline = ( - currentTeam: number, - appConfig: IConfig, - teamConfig?: ITeamConfig -) => { - return currentTeam === API_NO_TEAM_ID +const getDefaultMacOSDeadline = ({ + currentTeamId, + appConfig, + teamConfig, +}: GetDefaultFnParams) => { + return currentTeamId === API_NO_TEAM_ID ? appConfig?.mdm.macos_updates.deadline || "" : teamConfig?.mdm?.macos_updates.deadline || ""; }; -const getDefaultWindowsDeadlineDays = ( - currentTeam: number, - appConfig: IConfig, - teamConfig?: ITeamConfig -) => { - return currentTeam === API_NO_TEAM_ID +const getDefaultWindowsDeadlineDays = ({ + currentTeamId, + appConfig, + teamConfig, +}: GetDefaultFnParams) => { + return currentTeamId === API_NO_TEAM_ID ? appConfig.mdm.windows_updates.deadline_days?.toString() ?? "" : teamConfig?.mdm?.windows_updates.deadline_days?.toString() ?? ""; }; -const getDefaultWindowsGracePeriodDays = ( - currentTeam: number, - appConfig: IConfig, - teamConfig?: ITeamConfig -) => { - return currentTeam === API_NO_TEAM_ID +const getDefaultWindowsGracePeriodDays = ({ + currentTeamId, + appConfig, + teamConfig, +}: GetDefaultFnParams) => { + return currentTeamId === API_NO_TEAM_ID ? appConfig.mdm.windows_updates.grace_period_days?.toString() ?? "" : teamConfig?.mdm?.windows_updates.grace_period_days?.toString() ?? ""; }; +export const generateKey = (args: GetDefaultFnParams) => { + return ( + `${args.currentTeamId}-` + + `${getDefaultMacOSDeadline(args)}-` + + `${getDefaultMacOSVersion(args)}-` + + `${getDefaultWindowsDeadlineDays(args)}-` + + `${getDefaultWindowsGracePeriodDays(args)}` + ); +}; + interface ITargetSectionProps { + appConfig: IConfig; currentTeamId: number; - onSelectAccordionItem: (platform: OSUpdatesSupportedPlatform) => void; + teamConfig?: ITeamConfig; + selectedPlatform: OSUpdatesSupportedPlatform; + onSelectPlatform: (platform: OSUpdatesSupportedPlatform) => void; + refetchAppConfig: () => void; + refetchTeamConfig: () => void; } const TargetSection = ({ + appConfig, currentTeamId, - onSelectAccordionItem, + selectedPlatform, + teamConfig, + onSelectPlatform, + refetchAppConfig, + refetchTeamConfig, }: ITargetSectionProps) => { - const { config } = useContext(AppContext); + const isMacMdmEnabled = appConfig.mdm.enabled_and_configured; + const isWindowsMdmEnabled = appConfig.mdm.windows_enabled_and_configured; - // We make the call at this component as multiple children components need - // this data. - const { data: teamData, isLoading: isLoadingTeam } = useQuery< - ILoadTeamResponse, - Error, - ITeamConfig - >(["team-config", currentTeamId], () => teamsAPI.load(currentTeamId), { - refetchOnWindowFocus: false, - enabled: currentTeamId > APP_CONTEXT_NO_TEAM_ID, - select: (data) => data.team, + const defaultMacOSVersion = getDefaultMacOSVersion({ + currentTeamId, + appConfig, + teamConfig, }); - - if (!config) return null; - - const isMacMdmEnabled = config.mdm.enabled_and_configured; - const isWindowsMdmEnabled = config.mdm.windows_enabled_and_configured; - - // Loading state rendering - if (isLoadingTeam) { - return ; - } - - const defaultMacOSVersion = getDefaultMacOSVersion( + const defaultMacOSDeadline = getDefaultMacOSDeadline({ currentTeamId, - config, - teamData - ); - const defaultMacOSDeadline = getDefaultMacOSDeadline( + appConfig, + teamConfig, + }); + const defaultWindowsDeadlineDays = getDefaultWindowsDeadlineDays({ currentTeamId, - config, - teamData - ); - const defaultWindowsDeadlineDays = getDefaultWindowsDeadlineDays( + appConfig, + teamConfig, + }); + const defaultWindowsGracePeriodDays = getDefaultWindowsGracePeriodDays({ currentTeamId, - config, - teamData - ); - const defaultWindowsGracePeriodDays = getDefaultWindowsGracePeriodDays( - currentTeamId, - config, - teamData - ); + appConfig, + teamConfig, + }); const renderTargetForms = () => { if (isMacMdmEnabled && isWindowsMdmEnabled) { @@ -123,7 +120,10 @@ const TargetSection = ({ defaultMacOSDeadline={defaultMacOSDeadline} defaultWindowsDeadlineDays={defaultWindowsDeadlineDays} defaultWindowsGracePeriodDays={defaultWindowsGracePeriodDays} - onSelectAccordionItem={onSelectAccordionItem} + selectedPlatform={selectedPlatform} + onSelectPlatform={onSelectPlatform} + refetchAppConfig={refetchAppConfig} + refetchTeamConfig={refetchTeamConfig} /> ); } else if (isMacMdmEnabled) { @@ -132,6 +132,8 @@ const TargetSection = ({ currentTeamId={currentTeamId} defaultMinOsVersion={defaultMacOSVersion} defaultDeadline={defaultMacOSDeadline} + refetchAppConfig={refetchAppConfig} + refetchTeamConfig={refetchTeamConfig} /> ); } @@ -140,6 +142,8 @@ const TargetSection = ({ currentTeamId={currentTeamId} defaultDeadlineDays={defaultWindowsDeadlineDays} defaultGracePeriodDays={defaultWindowsGracePeriodDays} + refetchAppConfig={refetchAppConfig} + refetchTeamConfig={refetchTeamConfig} /> ); }; diff --git a/frontend/pages/ManageControlsPage/OSUpdates/components/WindowsTargetForm/WindowsTargetForm.tsx b/frontend/pages/ManageControlsPage/OSUpdates/components/WindowsTargetForm/WindowsTargetForm.tsx index 0374c71d24..ec7007a29e 100644 --- a/frontend/pages/ManageControlsPage/OSUpdates/components/WindowsTargetForm/WindowsTargetForm.tsx +++ b/frontend/pages/ManageControlsPage/OSUpdates/components/WindowsTargetForm/WindowsTargetForm.tsx @@ -1,6 +1,5 @@ import React, { useContext, useState } from "react"; import { isEmpty } from "lodash"; -import classnames from "classnames"; import { APP_CONTEXT_NO_TEAM_ID } from "interfaces/team"; import { NotificationContext } from "context/notification"; @@ -11,7 +10,6 @@ import teamsAPI from "services/entities/teams"; import InputField from "components/forms/fields/InputField"; import Button from "components/buttons/Button"; import validatePresence from "components/forms/validators/validate_presence"; -import { AppContext } from "context/app"; const baseClass = "windows-target-form"; @@ -86,16 +84,17 @@ interface IWindowsTargetFormProps { currentTeamId: number; defaultDeadlineDays: string; defaultGracePeriodDays: string; - inAccordion?: boolean; + refetchAppConfig: () => void; + refetchTeamConfig: () => void; } const WindowsTargetForm = ({ currentTeamId, defaultDeadlineDays, defaultGracePeriodDays, - inAccordion = false, + refetchAppConfig, + refetchTeamConfig, }: IWindowsTargetFormProps) => { - const { setConfig } = useContext(AppContext); const { renderFlash } = useContext(NotificationContext); const [isSaving, setIsSaving] = useState(false); const [deadlineDays, setDeadlineDays] = useState( @@ -111,15 +110,8 @@ const WindowsTargetForm = ({ string | undefined >(); - const classNames = classnames(baseClass, { - [`${baseClass}__accordion-form`]: inAccordion, - }); - - const updateNoTeamConfig = async (updateData: IWindowsMdmConfigData) => { - const updatedConfig = await configAPI.update(updateData); - setConfig(updatedConfig); - }; - + // FIXME: This behaves unexpectedly when a user switches tabs or changes the teams dropdown while the form is + // submitting because this component is unmounted. const handleSubmit = async (e: React.FormEvent) => { e.preventDefault(); const errors = validateForm({ @@ -135,7 +127,7 @@ const WindowsTargetForm = ({ const updateData = createMdmConfigData(deadlineDays, gracePeriodDays); try { currentTeamId === APP_CONTEXT_NO_TEAM_ID - ? await updateNoTeamConfig(updateData) + ? await configAPI.update(updateData) : await teamsAPI.update(updateData, currentTeamId); renderFlash( "success", @@ -144,6 +136,9 @@ const WindowsTargetForm = ({ } catch { renderFlash("error", "Couldn’t update. Please try again."); } finally { + currentTeamId === APP_CONTEXT_NO_TEAM_ID + ? refetchAppConfig() + : refetchTeamConfig(); setIsSaving(false); } } @@ -158,7 +153,7 @@ const WindowsTargetForm = ({ }; return ( -
+