From 5ed443590e2bb935e3ee450df034740eff067c92 Mon Sep 17 00:00:00 2001 From: RachelElysia <71795832+RachelElysia@users.noreply.github.com> Date: Wed, 4 Oct 2023 14:35:18 -0700 Subject: [PATCH] Fleet UI: Surface delete previous results modals (#14257) --- .../QueryDetailsPage/QueryDetailsPage.tsx | 4 +- .../edit/EditQueryPage/EditQueryPage.tsx | 7 ++- .../components/QueryForm/QueryForm.tests.tsx | 2 + .../edit/components/QueryForm/QueryForm.tsx | 47 ++++++++++++++----- .../SaveChangesModal/SaveChangesModal.tsx | 2 +- 5 files changed, 46 insertions(+), 16 deletions(-) diff --git a/frontend/pages/queries/details/QueryDetailsPage/QueryDetailsPage.tsx b/frontend/pages/queries/details/QueryDetailsPage/QueryDetailsPage.tsx index be52bd6ad3..17c46c3ade 100644 --- a/frontend/pages/queries/details/QueryDetailsPage/QueryDetailsPage.tsx +++ b/frontend/pages/queries/details/QueryDetailsPage/QueryDetailsPage.tsx @@ -1,4 +1,4 @@ -import React, { useContext, useEffect } from "react"; +import React, { useContext } from "react"; import { useQuery } from "react-query"; import { InjectedRouter, Params } from "react-router/lib/Router"; import { useErrorHandler } from "react-error-boundary"; @@ -112,7 +112,7 @@ const QueryDetailsPage = ({ ); const isLoading = isStoredQueryLoading; // TODO: Add || isCachedResultsLoading for new API response - const isApiError = storedQueryError || true; // TODO: Add || isCachedResultsError for new API response + const isApiError = storedQueryError || false; // TODO: Add || isCachedResultsError for new API response const renderHeader = () => { const canEditQuery = diff --git a/frontend/pages/queries/edit/EditQueryPage/EditQueryPage.tsx b/frontend/pages/queries/edit/EditQueryPage/EditQueryPage.tsx index e9aa57dce9..59b14feb72 100644 --- a/frontend/pages/queries/edit/EditQueryPage/EditQueryPage.tsx +++ b/frontend/pages/queries/edit/EditQueryPage/EditQueryPage.tsx @@ -100,13 +100,14 @@ const EditQueryPage = ({ const [showOpenSchemaActionText, setShowOpenSchemaActionText] = useState( false ); + const [showSaveChangesModal, setShowSaveChangesModal] = useState(false); // disabled on page load so we can control the number of renders // else it will re-populate the context on occasion const { isLoading: isStoredQueryLoading, data: storedQuery, - error: storedQueryError, + refetch: refetchStoredQuery, } = useQuery( ["query", queryId], () => queryAPI.load(queryId as number), @@ -215,6 +216,7 @@ const EditQueryPage = ({ try { await queryAPI.update(queryId, updatedQuery); renderFlash("success", "Query updated!"); + refetchStoredQuery(); // Required to compare recently saved query to a subsequent save to the query } catch (updateError: any) { console.error(updateError); if (updateError.data.errors[0].reason.includes("Duplicate")) { @@ -228,6 +230,7 @@ const EditQueryPage = ({ } setIsQueryUpdating(false); + setShowSaveChangesModal(false); // Closes conditionally opened modal when discarding previous results return false; }; @@ -304,6 +307,8 @@ const EditQueryPage = ({ isQuerySaving={isQuerySaving} isQueryUpdating={isQueryUpdating} hostId={parseInt(location.query.host_ids as string, 10)} + showSaveChangesModal={showSaveChangesModal} + setShowSaveChangesModal={setShowSaveChangesModal} /> diff --git a/frontend/pages/queries/edit/components/QueryForm/QueryForm.tests.tsx b/frontend/pages/queries/edit/components/QueryForm/QueryForm.tests.tsx index bb5286895f..f3655dd3f9 100644 --- a/frontend/pages/queries/edit/components/QueryForm/QueryForm.tests.tsx +++ b/frontend/pages/queries/edit/components/QueryForm/QueryForm.tests.tsx @@ -72,6 +72,8 @@ describe("QueryForm - component", () => { onOpenSchemaSidebar={jest.fn()} renderLiveQueryWarning={jest.fn()} backendValidators={{}} + showSaveChangesModal={false} + setShowSaveChangesModal={jest.fn()} /> ); diff --git a/frontend/pages/queries/edit/components/QueryForm/QueryForm.tsx b/frontend/pages/queries/edit/components/QueryForm/QueryForm.tsx index a704ee2ef8..f5d269b1d2 100644 --- a/frontend/pages/queries/edit/components/QueryForm/QueryForm.tsx +++ b/frontend/pages/queries/edit/components/QueryForm/QueryForm.tsx @@ -73,6 +73,8 @@ interface IQueryFormProps { renderLiveQueryWarning: () => JSX.Element | null; backendValidators: { [key: string]: string }; hostId?: number; + showSaveChangesModal: boolean; + setShowSaveChangesModal: (bool: boolean) => void; } const validateQuerySQL = (query: string) => { @@ -120,6 +122,8 @@ const QueryForm = ({ renderLiveQueryWarning, backendValidators, hostId, + showSaveChangesModal, + setShowSaveChangesModal, }: IQueryFormProps): JSX.Element => { // Note: The QueryContext values should always be used for any mutable query data such as query name // The storedQuery prop should only be used to access immutable metadata such as author id @@ -158,7 +162,6 @@ const QueryForm = ({ const savedQueryMode = !!queryIdForEdit; const [errors, setErrors] = useState<{ [key: string]: any }>({}); // string | null | undefined or boolean | undefined const [showSaveQueryModal, setShowSaveQueryModal] = useState(false); - const [showSaveChangesModal, setShowSaveChangesModal] = useState(false); // #7766 implementation const [showQueryEditor, setShowQueryEditor] = useState( isObserverPlus || isAnyTeamObserverPlus || false ); @@ -211,7 +214,6 @@ const QueryForm = ({ setShowSaveQueryModal(!showSaveQueryModal); }; - // #7766 implementation const toggleSaveChangesModal = () => { setShowSaveChangesModal(!showSaveChangesModal); }; @@ -411,12 +413,6 @@ const QueryForm = ({ logging: lastEditedQueryLoggingType, }); } - - // #7766 implementation - // savedQueryMode - // ? setShowSaveChangesModal(true) - // : setShowSaveQueryModal(true); - // TODO: onUpdate for saveChangesModal } }; @@ -609,6 +605,26 @@ const QueryForm = ({ const hasSavePermissions = isGlobalAdmin || isGlobalMaintainer; + const hasSqlChange = storedQuery && lastEditedQueryBody !== storedQuery.query; + const hasSnapshotChange = + storedQuery && + lastEditedQueryLoggingType !== "snapshot" && + storedQuery.logging === "snapshot"; + // Use commented out logic when discard data checkbox is implemented #13470 + const hasEnabledDiscardData = false; + // const hasEnabledDiscardData = + // storedQuery && lastEditedDiscardData && !storedQuery.discardData; + + const confirmChanges = (): boolean => { + // Confirm changes if the query has been edited, removed snapshot logging, or enabled discard data + return hasSqlChange || hasSnapshotChange || hasEnabledDiscardData; + }; + + const confirmSqlChange = (): boolean => { + // Confirm sql changes message if sql changed but snapshot and enabling discard data has not + return !!hasSqlChange && !hasSnapshotChange && !hasEnabledDiscardData; + }; + // Global admin, any maintainer, any observer+ on new query const renderEditableQueryForm = () => { // Save disabled for team maintainer/admins viewing global queries @@ -640,7 +656,9 @@ const QueryForm = ({ onLoad={onLoad} wrapperClassName={`${baseClass}__text-editor-wrapper`} onChange={onChangeQuery} - handleSubmit={promptSaveQuery} + handleSubmit={ + confirmChanges() ? toggleSaveChangesModal : promptSaveQuery + } wrapEnabled focus={!savedQueryMode} /> @@ -743,7 +761,11 @@ const QueryForm = ({