From 639aa29c4aaa25cbc0eff013bd779ceecb2de2cc Mon Sep 17 00:00:00 2001
From: Raphael Krut-Landau <raphael.kl@gmail.com>
Date: Thu, 11 Apr 2024 17:35:36 -0400
Subject: [PATCH] In Admin > Performance, display minimum query duration in
seconds (#40970)
---
.../StrategyEditorForDatabases.unit.spec.tsx | 4 +-
.../src/metabase-types/api/performance.ts | 1 +
.../components/StrategyEditorForDatabases.tsx | 10 ++++-
.../StrategyEditorForDatabases.unit.spec.tsx | 2 +-
.../performance/components/StrategyForm.tsx | 7 +++-
.../performance/hooks/useCacheConfigs.tsx | 6 ++-
.../metabase/admin/performance/strategies.ts | 38 +++++++++++++++++--
7 files changed, 55 insertions(+), 13 deletions(-)
diff --git a/enterprise/frontend/src/metabase-enterprise/caching/components/StrategyEditorForDatabases.unit.spec.tsx b/enterprise/frontend/src/metabase-enterprise/caching/components/StrategyEditorForDatabases.unit.spec.tsx
index e846565d387..a4c559e76fa 100644
--- a/enterprise/frontend/src/metabase-enterprise/caching/components/StrategyEditorForDatabases.unit.spec.tsx
+++ b/enterprise/frontend/src/metabase-enterprise/caching/components/StrategyEditorForDatabases.unit.spec.tsx
@@ -73,7 +73,7 @@ describe("StrategyEditorForDatabases", () => {
expect(await getSaveButton()).toBeInTheDocument();
await act(async () => {
- await changeInput(/minimum query duration/i, 60000, 70000);
+ await changeInput(/minimum query duration/i, 1, 5);
await changeInput(/multiplier/i, 10, 3);
});
@@ -136,7 +136,7 @@ describe("StrategyEditorForDatabases", () => {
expect((await screen.findAllByRole("spinbutton")).length).toBe(2);
await act(async () => {
- await changeInput(/minimum query duration/i, 60000, 70000);
+ await changeInput(/minimum query duration/i, 1, 5);
await changeInput(/multiplier/i, 10, 3);
});
diff --git a/frontend/src/metabase-types/api/performance.ts b/frontend/src/metabase-types/api/performance.ts
index 3a4730c9c9c..ffd7fab62c5 100644
--- a/frontend/src/metabase-types/api/performance.ts
+++ b/frontend/src/metabase-types/api/performance.ts
@@ -17,6 +17,7 @@ export interface TTLStrategy extends StrategyBase {
type: "ttl";
multiplier: number;
min_duration_ms: number;
+ min_duration_seconds?: number;
}
export interface DoNotCacheStrategy extends StrategyBase {
diff --git a/frontend/src/metabase/admin/performance/components/StrategyEditorForDatabases.tsx b/frontend/src/metabase/admin/performance/components/StrategyEditorForDatabases.tsx
index 1a3b5d2b393..4c115e34068 100644
--- a/frontend/src/metabase/admin/performance/components/StrategyEditorForDatabases.tsx
+++ b/frontend/src/metabase/admin/performance/components/StrategyEditorForDatabases.tsx
@@ -17,7 +17,12 @@ import { useCacheConfigs } from "../hooks/useCacheConfigs";
import { useConfirmOnRouteLeave } from "../hooks/useConfirmOnRouteLeave";
import { useVerticallyOverflows } from "../hooks/useVerticallyOverflows";
import type { UpdateTargetId } from "../strategies";
-import { getFieldsForStrategyType, rootId, Strategies } from "../strategies";
+import {
+ getFieldsForStrategyType,
+ rootId,
+ Strategies,
+ translateConfigToAPI,
+} from "../strategies";
import { Panel, TabWrapper } from "./StrategyEditorForDatabases.styled";
import { StrategyForm } from "./StrategyForm";
@@ -137,7 +142,8 @@ const StrategyEditorForDatabases_Base = ({
strategy: validatedStrategy,
};
- await CacheConfigApi.update(newConfig);
+ const translatedConfig = translateConfigToAPI(newConfig);
+ await CacheConfigApi.update(translatedConfig);
setConfigs([...otherConfigs, newConfig]);
}
},
diff --git a/frontend/src/metabase/admin/performance/components/StrategyEditorForDatabases.unit.spec.tsx b/frontend/src/metabase/admin/performance/components/StrategyEditorForDatabases.unit.spec.tsx
index 136ec297d22..674b24c3bdd 100644
--- a/frontend/src/metabase/admin/performance/components/StrategyEditorForDatabases.unit.spec.tsx
+++ b/frontend/src/metabase/admin/performance/components/StrategyEditorForDatabases.unit.spec.tsx
@@ -19,7 +19,7 @@ describe("StrategyEditorForDatabases", () => {
expect(await getSaveButton()).toBeInTheDocument();
await act(async () => {
- await changeInput(/minimum query duration/i, 60000, 70000);
+ await changeInput(/minimum query duration/i, 1, 5);
await changeInput(/multiplier/i, 10, 3);
});
diff --git a/frontend/src/metabase/admin/performance/components/StrategyForm.tsx b/frontend/src/metabase/admin/performance/components/StrategyForm.tsx
index 2d83fb556cb..d8982a84ac4 100644
--- a/frontend/src/metabase/admin/performance/components/StrategyForm.tsx
+++ b/frontend/src/metabase/admin/performance/components/StrategyForm.tsx
@@ -104,9 +104,12 @@ const StrategyFormBody = ({
<>
<Field
title={t`Minimum query duration`}
- subtitle={t`Metabase will cache all saved questions with an average query execution time greater than this many milliseconds.`}
+ subtitle={t`Metabase will cache all saved questions with an average query execution time greater than this many seconds.`}
>
- <PositiveNumberInput strategyType="ttl" name="min_duration_ms" />
+ <PositiveNumberInput
+ strategyType="ttl"
+ name="min_duration_seconds"
+ />
</Field>
<Field
title={t`Cache time-to-live (TTL) multiplier`}
diff --git a/frontend/src/metabase/admin/performance/hooks/useCacheConfigs.tsx b/frontend/src/metabase/admin/performance/hooks/useCacheConfigs.tsx
index d1d29e08b87..fd0cdade7d2 100644
--- a/frontend/src/metabase/admin/performance/hooks/useCacheConfigs.tsx
+++ b/frontend/src/metabase/admin/performance/hooks/useCacheConfigs.tsx
@@ -5,7 +5,7 @@ import { useDatabaseListQuery } from "metabase/common/hooks";
import { CacheConfigApi } from "metabase/services";
import type { CacheConfigAPIResponse, Config } from "metabase-types/api";
-import { rootId } from "../strategies";
+import { rootId, translateConfigFromAPI } from "../strategies";
import { useRecentlyTrue } from "./useRecentlyTrue";
@@ -28,7 +28,9 @@ export const useCacheConfigs = ({
})) as CacheConfigAPIResponse
).data
: [];
- const configs = [...rootConfigsFromAPI, ...dbConfigsFromAPI];
+ const configs = [...rootConfigsFromAPI, ...dbConfigsFromAPI].map(config =>
+ translateConfigFromAPI(config),
+ );
return configs;
}, []);
diff --git a/frontend/src/metabase/admin/performance/strategies.ts b/frontend/src/metabase/admin/performance/strategies.ts
index 7e4307c549b..2bb3edf3bee 100644
--- a/frontend/src/metabase/admin/performance/strategies.ts
+++ b/frontend/src/metabase/admin/performance/strategies.ts
@@ -3,7 +3,7 @@ import type { AnySchema } from "yup";
import * as Yup from "yup";
import type { SchemaObjectDescription } from "yup/lib/schema";
-import type { Strategy, StrategyType } from "metabase-types/api";
+import type { Config, Strategy, StrategyType } from "metabase-types/api";
import { DurationUnit } from "metabase-types/api";
export type UpdateTargetId = (
@@ -22,8 +22,8 @@ export const rootId = 0;
const durationUnits = new Set(Object.values(DurationUnit).map(String));
const positiveInteger = Yup.number()
- .positive(t`The minimum query duration must be a positive number.`)
- .integer(t`The minimum query duration must be an integer.`);
+ .positive(t`Enter a positive number.`)
+ .integer(t`Enter an integer.`);
export const inheritStrategyValidationSchema = Yup.object({
type: Yup.string().equals(["inherit"]),
@@ -33,9 +33,13 @@ export const doNotCacheStrategyValidationSchema = Yup.object({
type: Yup.string().equals(["nocache"]),
});
+export const defaultMinDurationMs = 1000;
export const ttlStrategyValidationSchema = Yup.object({
type: Yup.string().equals(["ttl"]),
- min_duration_ms: positiveInteger.default(60000),
+ min_duration_ms: positiveInteger.default(defaultMinDurationMs),
+ min_duration_seconds: positiveInteger.default(
+ Math.ceil(defaultMinDurationMs / 1000),
+ ),
multiplier: positiveInteger.default(10),
});
@@ -126,3 +130,29 @@ export const getFieldsForStrategyType = (strategyType: StrategyType) => {
const fields = Object.keys(fieldRecord);
return fields;
};
+
+export const translateConfig = (
+ config: Config,
+ direction: "fromAPI" | "toAPI",
+): Config => {
+ const translated: Config = { ...config };
+ if (translated.strategy.type === "ttl") {
+ if (direction === "fromAPI") {
+ translated.strategy.min_duration_seconds = Math.ceil(
+ translated.strategy.min_duration_ms / 1000,
+ );
+ } else {
+ translated.strategy.min_duration_ms =
+ translated.strategy.min_duration_seconds === undefined
+ ? defaultMinDurationMs
+ : translated.strategy.min_duration_seconds * 1000;
+ delete translated.strategy.min_duration_seconds;
+ }
+ }
+ return translated;
+};
+
+export const translateConfigFromAPI = (config: Config): Config =>
+ translateConfig(config, "fromAPI");
+export const translateConfigToAPI = (config: Config): Config =>
+ translateConfig(config, "toAPI");
--
GitLab