From 40b8545400697194aba4d02336b531fef40400d8 Mon Sep 17 00:00:00 2001 From: Nate Kelley Date: Wed, 16 Apr 2025 13:49:42 -0600 Subject: [PATCH] metric save version --- .../api/buster_rest/metrics/queryRequests.ts | 15 ++++-- .../versionHistory/VersionHistoryPanel.tsx | 18 +++++-- .../useListVersionHistories.tsx | 34 ++++++------- .../BusterAppLayout/AppLayoutProvider.tsx | 48 ++++++++++++------- .../context/Metrics/useIsMetricChanged.tsx | 6 ++- .../VersionHistoryHeaderButtons.tsx | 25 ++++++---- .../useCloseVersionHistory.ts | 2 - 7 files changed, 95 insertions(+), 53 deletions(-) diff --git a/web/src/api/buster_rest/metrics/queryRequests.ts b/web/src/api/buster_rest/metrics/queryRequests.ts index 392ea6ebb..443aac4c7 100644 --- a/web/src/api/buster_rest/metrics/queryRequests.ts +++ b/web/src/api/buster_rest/metrics/queryRequests.ts @@ -264,12 +264,19 @@ export const useSaveMetric = (params?: { updateOnSave?: boolean }) => { metricsQueryKeys.metricsGetData(id, restore_to_version).queryKey ); if (oldMetric && newMetric && newMetricData) { - console.log('TODO. look into this because I do not think this is correct'); + const latestVersionNumber = queryClient.getQueryData( + metricsQueryKeys.metricsGetMetric(id, undefined).queryKey + )?.version_number; + const newVersionNumber = (latestVersionNumber || 0) + 1; + queryClient.setQueryData( - metricsQueryKeys.metricsGetMetric(id, versionNumber).queryKey, + metricsQueryKeys.metricsGetMetric(id, newVersionNumber).queryKey, oldMetric ); - queryClient.setQueryData(metricsQueryKeys.metricsGetData(id).queryKey, newMetricData); + queryClient.setQueryData( + metricsQueryKeys.metricsGetData(id, newVersionNumber).queryKey, + newMetricData + ); } } @@ -302,7 +309,7 @@ export const useSaveMetric = (params?: { updateOnSave?: boolean }) => { newMetric ); //We need to update BOTH the versioned and the non-versioned metric for version updates to keep the latest up to date - if (variables.update_version) { + if (variables.update_version || variables.restore_to_version) { queryClient.setQueryData( metricsQueryKeys.metricsGetMetric(data.id, undefined).queryKey, newMetric diff --git a/web/src/components/features/versionHistory/VersionHistoryPanel.tsx b/web/src/components/features/versionHistory/VersionHistoryPanel.tsx index d424626c0..e5ac4100c 100644 --- a/web/src/components/features/versionHistory/VersionHistoryPanel.tsx +++ b/web/src/components/features/versionHistory/VersionHistoryPanel.tsx @@ -14,6 +14,7 @@ import Link from 'next/link'; import { useGetFileLink } from '@/context/Assets/useGetFileLink'; import { useChatLayoutContextSelector } from '@/layouts/ChatLayout'; import { useCallback } from 'react'; +import { CircleSpinnerLoader } from '@/components/ui/loaders'; export const VersionHistoryPanel = React.memo( ({ assetId, type }: { assetId: string; type: 'metric' | 'dashboard' }) => { @@ -21,6 +22,7 @@ export const VersionHistoryPanel = React.memo( const { listItems, onPrefetchAsset, + restoringVersion, currentVersionNumber, selectedQueryVersion, onClickRestoreVersion @@ -61,6 +63,7 @@ export const VersionHistoryPanel = React.memo( selected={item.version_number === selectedQueryVersion} showRestoreButton={item.version_number !== currentVersionNumber} onClickRestoreVersion={onClickRestoreVersion} + restoringVersion={restoringVersion} link={ getFileLink({ fileId: assetId, @@ -85,6 +88,7 @@ const ListItem = React.memo( selected, showRestoreButton, link, + restoringVersion, onClickRestoreVersion, onPrefetchAsset }: { @@ -92,6 +96,7 @@ const ListItem = React.memo( updated_at: string; selected: boolean; showRestoreButton: boolean; + restoringVersion: number | null; onClickRestoreVersion: (versionNumber: number) => void; onPrefetchAsset: (versionNumber: number, link: string) => Promise; link: string; @@ -111,6 +116,8 @@ const ListItem = React.memo( } }, []); + const isRestoringVersion = restoringVersion === version_number; + return (
{showRestoreButton && ( - +
{ + if (restoringVersion) return; + e.stopPropagation(); e.preventDefault(); onClickRestoreVersion(version_number); }} - className="hover:bg-gray-light/20 hover:text-foreground -mr-1 rounded p-1 opacity-0 group-hover:block group-hover:opacity-100"> - + className={cn( + 'hover:bg-gray-light/20 hover:text-foreground -mr-1 rounded p-1 opacity-0 group-hover:block group-hover:opacity-100', + isRestoringVersion && 'cursor-not-allowed opacity-100!' + )}> + {isRestoringVersion ? : }
)} diff --git a/web/src/components/features/versionHistory/useListVersionHistories.tsx b/web/src/components/features/versionHistory/useListVersionHistories.tsx index 51b9db795..e3a1dc0ea 100644 --- a/web/src/components/features/versionHistory/useListVersionHistories.tsx +++ b/web/src/components/features/versionHistory/useListVersionHistories.tsx @@ -16,9 +16,10 @@ import { useChatLayoutContextSelector } from '@/layouts/ChatLayout'; import { useCloseVersionHistory } from '@/layouts/ChatLayout/FileContainer/FileContainerHeader/FileContainerHeaderVersionHistory'; import { BusterRoutes, createBusterRoute } from '@/routes'; import last from 'lodash/last'; -import { useMemo } from 'react'; +import { useMemo, useState } from 'react'; import { usePrefetchGetMetricClient } from '@/api/buster_rest/metrics'; import { useRouter } from 'next/navigation'; +import { timeout } from '@/lib/timeout'; export const useListVersionHistories = ({ assetId, @@ -29,6 +30,7 @@ export const useListVersionHistories = ({ }) => { const router = useRouter(); const { onCloseVersionHistory } = useCloseVersionHistory(); + const [restoringVersion, setRestoringVersion] = useState(null); const onChangePage = useAppLayoutContextSelector((x) => x.onChangePage); const { versions: dashboardVersions, @@ -68,6 +70,8 @@ export const useListVersionHistories = ({ const onClickRestoreVersion = useMemoizedFn( async (versionNumber: number, rereouteToAsset: boolean = true) => { + setRestoringVersion(versionNumber); + if (type === 'metric') { await onRestoreMetricVersion(versionNumber); if (rereouteToAsset) { @@ -92,8 +96,9 @@ export const useListVersionHistories = ({ onCloseVersionHistory(); } } - onCloseVersionHistory(); + await timeout(500); + setRestoringVersion(null); } ); @@ -107,24 +112,19 @@ export const useListVersionHistories = ({ } }); - return useMemo(() => { - return { - listItems, - currentVersionNumber, - selectedQueryVersion, - onClickRestoreVersion, - isRestoringVersion: isSavingDashboard || isSavingMetric, - onPrefetchAsset - }; - }, [ + const isRestoringVersion = useMemo(() => { + return isSavingDashboard || isSavingMetric || restoringVersion !== null; + }, [isSavingDashboard, isSavingMetric, restoringVersion]); + + return { listItems, currentVersionNumber, selectedQueryVersion, onClickRestoreVersion, - isSavingDashboard, - isSavingMetric, + isRestoringVersion, + restoringVersion, onPrefetchAsset - ]); + }; }; type UseListVersionReturn = { @@ -219,9 +219,11 @@ const useListMetricVersions = ({ const { data: metric } = useGetMetric( { - id: type === 'metric' ? assetId : undefined + id: type === 'metric' ? assetId : undefined, + versionNumber: null }, { + enabled: !!assetId, //we do not want to have undefined versions when select: (x) => ({ versions: x.versions, version_number: x.version_number diff --git a/web/src/context/BusterAppLayout/AppLayoutProvider.tsx b/web/src/context/BusterAppLayout/AppLayoutProvider.tsx index aa4c6725a..19fe17c89 100644 --- a/web/src/context/BusterAppLayout/AppLayoutProvider.tsx +++ b/web/src/context/BusterAppLayout/AppLayoutProvider.tsx @@ -21,12 +21,14 @@ export const useAppLayout = () => { ): Promise => { const targetPath = typeof params === 'string' ? params : createBusterRoute(params); - if (options?.shallow) { - const isSameMinusQueryParams = - new URL(targetPath, window.location.origin).pathname === - new URL(window.location.href).pathname; + // Extract the pathname without query parameters + const targetPathname = new URL(targetPath, window.location.origin).pathname; + const currentPathname = new URL(window.location.href).pathname; - if (isSameMinusQueryParams) { + if (options?.shallow) { + const isSamePathname = targetPathname === currentPathname; + + if (isSamePathname) { return new Promise((resolve) => { const params = getQueryParamsFromPath(targetPath); onChangeQueryParams(params, false); @@ -36,33 +38,45 @@ export const useAppLayout = () => { } return new Promise((resolve) => { - const currentPath = window.location.pathname; - - // If we're already on the target path, resolve immediately - if (currentPath === targetPath) { + // If we're already on the target pathname, but query params might differ + if (currentPathname === targetPathname && targetPath.indexOf('?') === -1) { + // Clear query params by using the pathname only + window.history.pushState({}, '', targetPathname); resolve(); return; } - // Set up an effect to watch for pathname changes + // If target and current pathnames are the same but target includes query params + if (currentPathname === targetPathname && targetPath.indexOf('?') !== -1) { + push(targetPath); + // Set up an effect to watch for pathname changes + const checkPathChange = (waitTime: number = 25, iteration: number = 0) => { + if (window.location.href.includes(targetPath)) { + resolve(); + } else if (iteration >= 10) { + resolve(); + } else { + const newWaitTime = waitTime * 1.25; + setTimeout(() => checkPathChange(newWaitTime, iteration + 1), newWaitTime); + } + }; + checkPathChange(); + return; + } + + // Default case - different pathnames const checkPathChange = (waitTime: number = 25, iteration: number = 0) => { - if (window.location.pathname !== currentPath) { + if (window.location.pathname !== currentPathname) { resolve(); } else if (iteration >= 10) { - // Resolve after 10 attempts to prevent infinite loops resolve(); } else { - // Check again in a short while if the path hasn't changed yet const newWaitTime = waitTime * 1.25; setTimeout(() => checkPathChange(newWaitTime, iteration + 1), newWaitTime); } }; - // Start the navigation push(targetPath); - console.log('pushed', targetPath); - - // Start checking for path changes checkPathChange(); }); } diff --git a/web/src/context/Metrics/useIsMetricChanged.tsx b/web/src/context/Metrics/useIsMetricChanged.tsx index e45035fdc..0f78d4092 100644 --- a/web/src/context/Metrics/useIsMetricChanged.tsx +++ b/web/src/context/Metrics/useIsMetricChanged.tsx @@ -23,7 +23,8 @@ export const useIsMetricChanged = ({ metricId }: { metricId: string }) => { name: x.name, description: x.description, chart_config: x.chart_config, - file: x.file + file: x.file, + version_number: x.version_number }) } ); @@ -44,7 +45,8 @@ export const useIsMetricChanged = ({ metricId }: { metricId: string }) => { 'name', 'description', 'chart_config', - 'file' + 'file', + 'version_number' ]), [originalMetric, currentMetric] ); diff --git a/web/src/layouts/ChatLayout/FileContainer/FileContainerHeader/FileContainerHeaderVersionHistory/VersionHistoryHeaderButtons.tsx b/web/src/layouts/ChatLayout/FileContainer/FileContainerHeader/FileContainerHeaderVersionHistory/VersionHistoryHeaderButtons.tsx index 22307550a..5748fe27b 100644 --- a/web/src/layouts/ChatLayout/FileContainer/FileContainerHeader/FileContainerHeaderVersionHistory/VersionHistoryHeaderButtons.tsx +++ b/web/src/layouts/ChatLayout/FileContainer/FileContainerHeader/FileContainerHeaderVersionHistory/VersionHistoryHeaderButtons.tsx @@ -3,20 +3,25 @@ import { Button } from '@/components/ui/buttons'; import { useChatLayoutContextSelector } from '@/layouts/ChatLayout'; import first from 'lodash/first'; import { History } from '@/components/ui/icons'; -import { useMemoizedFn } from '@/hooks'; +import { useMemoizedFn, useMount } from '@/hooks'; import React from 'react'; import { useListVersionDropdownItems } from '@/components/features/versionHistory/useListVersionDropdownItems'; import { FileType } from '@/api/asset_interfaces/chat'; import { Dropdown } from '@/components/ui/dropdown'; -export const VersionHistoryHeaderButtons: React.FC<{}> = ({}) => { +export const VersionHistoryHeaderButtons: React.FC<{}> = React.memo(({}) => { const selectedFile = useChatLayoutContextSelector((x) => x.selectedFile); const chatId = useChatLayoutContextSelector((x) => x.chatId); - const { listItems, isRestoringVersion, selectedQueryVersion, onClickRestoreVersion } = - useListVersionHistories({ - assetId: selectedFile?.id || '', - type: selectedFile?.type as 'metric' | 'dashboard' - }); + const { + listItems, + restoringVersion, + isRestoringVersion, + selectedQueryVersion, + onClickRestoreVersion + } = useListVersionHistories({ + assetId: selectedFile?.id || '', + type: selectedFile?.type as 'metric' | 'dashboard' + }); const currentVersion = first(listItems)?.version_number; const isSelectedVersionCurrent = selectedQueryVersion === currentVersion; @@ -41,11 +46,13 @@ export const VersionHistoryHeaderButtons: React.FC<{}> = ({}) => { disabled={isSelectedVersionCurrent || !currentVersion} onClick={onClickRestoreVersionPreflight} loading={isRestoringVersion}> - Restore version + Restore version {restoringVersion}
); -}; +}); + +VersionHistoryHeaderButtons.displayName = 'VersionHistoryHeaderButtons'; const VersionSelectButton = React.memo( ({ diff --git a/web/src/layouts/ChatLayout/FileContainer/FileContainerHeader/FileContainerHeaderVersionHistory/useCloseVersionHistory.ts b/web/src/layouts/ChatLayout/FileContainer/FileContainerHeader/FileContainerHeaderVersionHistory/useCloseVersionHistory.ts index c88083669..4ab7e8db7 100644 --- a/web/src/layouts/ChatLayout/FileContainer/FileContainerHeader/FileContainerHeaderVersionHistory/useCloseVersionHistory.ts +++ b/web/src/layouts/ChatLayout/FileContainer/FileContainerHeader/FileContainerHeaderVersionHistory/useCloseVersionHistory.ts @@ -7,7 +7,6 @@ import { useMemo } from 'react'; export const useCloseVersionHistory = () => { const onChangePage = useAppLayoutContextSelector((x) => x.onChangePage); - const closeSecondaryView = useChatLayoutContextSelector((x) => x.closeSecondaryView); const chatId = useChatLayoutContextSelector((x) => x.chatId); const metricId = useChatLayoutContextSelector((x) => x.metricId); const dashboardId = useChatLayoutContextSelector((x) => x.dashboardId); @@ -37,7 +36,6 @@ export const useCloseVersionHistory = () => { const onCloseVersionHistory = useMemoizedFn(() => { onChangePage(href); - closeSecondaryView(); }); return { href, onCloseVersionHistory };