From 34802667ec8f69d8277664d4d1b3f4a8dbc57fc6 Mon Sep 17 00:00:00 2001 From: jpople Date: Wed, 20 Nov 2024 12:22:49 -0600 Subject: [PATCH] Handle timeouts in monitor queries in the UI (#5519) --- CHANGELOG.md | 4 ++ .../ConfigureMonitorDatabasesForm.tsx | 26 ++++++++++++- .../ConfigureMonitorModal.tsx | 33 +++++++++++++--- .../useCumulativeGetDatabases.tsx | 38 +++++++++++++++++-- 4 files changed, 90 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f6ca8cfc9..25af067ef3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ The types of changes are: - The CMP override `fides_privacy_policy_url` will now apply even if the `fides_override_language` doesn't match [#5515](https://github.com/ethyca/fides/pull/5515) - Updated POST taxonomy endpoints to handle creating resources without specifying fides_key [#5468](https://github.com/ethyca/fides/pull/5468) - Disabled connection pooling for task workers and added retries and keep-alive configurations for database connections [#5448](https://github.com/ethyca/fides/pull/5448) +- Added timeout handling in the UI for async discovery monitor-related queries [#5519](https://github.com/ethyca/fides/pull/5519) ### Developer Experience - Migrated several instances of Chakra's Select component to use Ant's Select component [#5475](https://github.com/ethyca/fides/pull/5475) @@ -45,6 +46,9 @@ The types of changes are: - Fixing issue where "privacy request received" emails would not be sent if the request had custom identities [#5518](https://github.com/ethyca/fides/pull/5518) - Fixed issue with long-running privacy request tasks losing their connection to the database [#5500](https://github.com/ethyca/fides/pull/5500) +### Docs +- Added docs for PrivacyNoticeRegion type [#5488](https://github.com/ethyca/fides/pull/5488) + ### Security - Password Policy is now Enforced in Accept Invite API [CVE-2024-52008](https://github.com/ethyca/fides/security/advisories/GHSA-v7vm-rhmg-8j2r) diff --git a/clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorDatabasesForm.tsx b/clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorDatabasesForm.tsx index fa2630cc33..1609eb2e9f 100644 --- a/clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorDatabasesForm.tsx +++ b/clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorDatabasesForm.tsx @@ -1,13 +1,18 @@ -import { AntButton as Button, Flex, Text, Tooltip } from "fidesui"; +import { AntButton as Button, Flex, Text, Tooltip, useToast } from "fidesui"; +import FidesSpinner from "~/features/common/FidesSpinner"; import { usePaginatedPicker } from "~/features/common/hooks/usePicker"; import QuestionTooltip from "~/features/common/QuestionTooltip"; +import { DEFAULT_TOAST_PARAMS } from "~/features/common/toast"; import MonitorDatabasePicker from "~/features/integrations/configure-monitor/MonitorDatabasePicker"; import useCumulativeGetDatabases from "~/features/integrations/configure-monitor/useCumulativeGetDatabases"; import { MonitorConfig } from "~/types/api"; const TOOLTIP_COPY = "Selecting a project will monitor all current and future datasets within that project."; +const TIMEOUT_COPY = + "Loading resources is taking longer than expected. The monitor has been saved and is tracking all available resources. You can return later to limit its scope if needed"; + const ConfigureMonitorDatabasesForm = ({ monitor, isEditing, @@ -23,13 +28,26 @@ const ConfigureMonitorDatabasesForm = ({ onSubmit: (monitor: MonitorConfig) => void; onClose: () => void; }) => { + const toast = useToast(); + + const handleTimeout = () => { + onSubmit({ ...monitor, databases: [] }); + toast({ + ...DEFAULT_TOAST_PARAMS, + status: "info", + description: TIMEOUT_COPY, + }); + onClose(); + }; + const { databases, totalDatabases: totalRows, fetchMore, reachedEnd, isLoading: refetchPending, - } = useCumulativeGetDatabases(integrationKey); + initialIsLoading, + } = useCumulativeGetDatabases(integrationKey, handleTimeout); const initialSelected = monitor?.databases ?? []; @@ -58,6 +76,10 @@ const ConfigureMonitorDatabasesForm = ({ const saveIsDisabled = !allSelected && selected.length === 0; + if (initialIsLoading) { + return ; + } + return ( <> diff --git a/clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorModal.tsx b/clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorModal.tsx index 5220988ef5..191ddb3a53 100644 --- a/clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorModal.tsx +++ b/clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorModal.tsx @@ -1,8 +1,9 @@ -import { UseDisclosureReturn } from "fidesui"; +import { UseDisclosureReturn, useToast } from "fidesui"; import FidesSpinner from "~/features/common/FidesSpinner"; import useQueryResultToast from "~/features/common/form/useQueryResultToast"; import FormModal from "~/features/common/modals/FormModal"; +import { DEFAULT_TOAST_PARAMS } from "~/features/common/toast"; import { useGetDatabasesByConnectionQuery, usePutDiscoveryMonitorMutation, @@ -14,7 +15,11 @@ import { ConnectionSystemTypeMap, MonitorConfig, } from "~/types/api"; -import { isErrorResult } from "~/types/errors"; +import { isErrorResult, RTKResult } from "~/types/errors"; + +const TIMEOUT_DELAY = 5000; +const TIMEOUT_COPY = + "Saving this monitor is taking longer than expected. Fides will continue processing it in the background, and you can check back later to view the updates."; const ConfigureMonitorModal = ({ isOpen, @@ -44,6 +49,8 @@ const ConfigureMonitorModal = ({ const databasesAvailable = !!databases && !!databases.total; + const toast = useToast(); + const { toastResult } = useQueryResultToast({ defaultSuccessMsg: `Monitor ${ isEditing ? "updated" : "created" @@ -54,10 +61,24 @@ const ConfigureMonitorModal = ({ }); const handleSubmit = async (values: MonitorConfig) => { - const result = await putMonitorMutationTrigger(values); - toastResult(result); - if (!isErrorResult(result)) { - onClose(); + let result: RTKResult | undefined; + const timeout = setTimeout(() => { + if (!result) { + toast({ + ...DEFAULT_TOAST_PARAMS, + status: "info", + description: TIMEOUT_COPY, + }); + onClose(); + } + }, TIMEOUT_DELAY); + result = await putMonitorMutationTrigger(values); + if (result) { + clearTimeout(timeout); + toastResult(result); + if (!isErrorResult(result)) { + onClose(); + } } }; diff --git a/clients/admin-ui/src/features/integrations/configure-monitor/useCumulativeGetDatabases.tsx b/clients/admin-ui/src/features/integrations/configure-monitor/useCumulativeGetDatabases.tsx index 3c4e2c9138..642f1d84d2 100644 --- a/clients/admin-ui/src/features/integrations/configure-monitor/useCumulativeGetDatabases.tsx +++ b/clients/admin-ui/src/features/integrations/configure-monitor/useCumulativeGetDatabases.tsx @@ -1,10 +1,12 @@ -import { useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { useGetDatabasesByConnectionQuery, useLazyGetDatabasesByConnectionQuery, } from "~/features/data-discovery-and-detection/discovery-detection.slice"; +const TIMEOUT_DELAY = 5000; + const EMPTY_RESPONSE = { items: [] as string[], total: 1, @@ -13,8 +15,12 @@ const EMPTY_RESPONSE = { pages: 0, }; -const useCumulativeGetDatabases = (integrationKey: string) => { +const useCumulativeGetDatabases = ( + integrationKey: string, + onTimeout?: () => void, +) => { const [nextPage, setNextPage] = useState(2); + const { data: initialResult, isLoading: initialIsLoading } = useGetDatabasesByConnectionQuery({ page: 1, @@ -22,6 +28,8 @@ const useCumulativeGetDatabases = (integrationKey: string) => { connection_config_key: integrationKey, }); + const initialLoadingRef = useRef(initialIsLoading); + const { items: initialDatabases, total: totalDatabases } = initialResult ?? EMPTY_RESPONSE; @@ -29,6 +37,23 @@ const useCumulativeGetDatabases = (integrationKey: string) => { const [databases, setDatabases] = useState(initialDatabases); + useEffect(() => { + initialLoadingRef.current = initialIsLoading; + // this needs to be in this hook or else it will be set to [] instead of the actual result + setDatabases(initialDatabases); + }, [initialIsLoading, initialDatabases]); + + useEffect(() => { + const t = setTimeout(() => { + if (initialLoadingRef.current && onTimeout) { + onTimeout(); + } + }, TIMEOUT_DELAY); + return () => clearTimeout(t); + // this should only run once on mount + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + const [ refetchTrigger, { isLoading: refetchIsLoading, isFetching: refetchIsFetching }, @@ -52,7 +77,14 @@ const useCumulativeGetDatabases = (integrationKey: string) => { setDatabases([...databases, ...(result.data?.items ?? [])]); }; - return { databases, totalDatabases, fetchMore, isLoading, reachedEnd }; + return { + databases, + totalDatabases, + fetchMore, + initialIsLoading, + isLoading, + reachedEnd, + }; }; export default useCumulativeGetDatabases;