Skip to content

Commit ac9ef74

Browse files
skynetigorskynetigor
authored
fix: Saved preset not working on UI (#4880)
1 parent 811b505 commit ac9ef74

File tree

11 files changed

+307
-55
lines changed

11 files changed

+307
-55
lines changed

‎keep-ui/components/LinkWithIcon.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ export const LinkWithIcon = ({
8080
)}
8181
onMouseEnter={handleMouseEnter}
8282
onMouseLeave={handleMouseLeave}
83+
data-testid={`${testId}-link-container`}
8384
>
8485
<Link
8586
tabIndex={tabIndex}

‎keep-ui/features/cel-input/__tests__/use-cel-state.test.ts

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
import { usePathname, useRouter, useSearchParams } from "next/navigation";
2+
import { useCelState } from "../use-cel-state";
3+
import { renderHook, act } from "@testing-library/react";
4+
5+
jest.mock("next/navigation", () => ({
6+
useRouter: jest.fn(),
7+
useSearchParams: jest.fn(),
8+
usePathname: jest.fn(() => "/alerts/feed"),
9+
}));
10+
jest.useFakeTimers();
11+
12+
describe("useCelState", () => {
13+
let replaceMock: jest.Mock;
14+
beforeEach(() => {
15+
(useSearchParams as jest.Mock).mockReturnValue(new URLSearchParams());
16+
replaceMock = jest.fn();
17+
(useRouter as jest.Mock).mockReturnValue({
18+
replace: replaceMock,
19+
});
20+
});
21+
22+
it("should initialize with defaultCel when no query param is present", () => {
23+
(useSearchParams as jest.Mock).mockReturnValue(new URLSearchParams({}));
24+
const { result } = renderHook(() =>
25+
useCelState({
26+
enableQueryParams: false,
27+
defaultCel: "name.contains('cpu')",
28+
})
29+
);
30+
31+
expect(result.current[0]).toBe("name.contains('cpu')");
32+
});
33+
34+
it("should initialize with query param value if present", () => {
35+
(useSearchParams as jest.Mock).mockReturnValue(
36+
new URLSearchParams({
37+
cel: "name.contains('cpu')",
38+
})
39+
);
40+
41+
const { result } = renderHook(() =>
42+
useCelState({
43+
enableQueryParams: true,
44+
defaultCel: "name.contains('memory')",
45+
})
46+
);
47+
48+
expect(result.current[0]).toBe("name.contains('cpu')");
49+
});
50+
51+
it("should update query params when celState changes and enableQueryParams is true", () => {
52+
(useSearchParams as jest.Mock).mockReturnValue(
53+
new URLSearchParams({
54+
cel: "name.contains('cpu')",
55+
})
56+
);
57+
58+
const { result } = renderHook(() =>
59+
useCelState({ enableQueryParams: true, defaultCel: "" })
60+
);
61+
62+
act(() => {
63+
result.current[1]("name.contains('memory')");
64+
});
65+
66+
act(() => {
67+
jest.advanceTimersByTime(500);
68+
});
69+
70+
expect(replaceMock).toHaveBeenCalledWith(
71+
"/?cel=name.contains%28%27memory%27%29"
72+
);
73+
});
74+
75+
describe("when enableQueryParams is false", () => {
76+
it("should not update query params", () => {
77+
const { result } = renderHook(() =>
78+
useCelState({ enableQueryParams: false, defaultCel: "" })
79+
);
80+
81+
act(() => {
82+
result.current[1]("name.contains('cpu')");
83+
});
84+
85+
expect(replaceMock).not.toHaveBeenCalled();
86+
});
87+
88+
it("should not have initial state from queryparams", () => {
89+
(useSearchParams as jest.Mock).mockReturnValue(
90+
new URLSearchParams({
91+
cel: "name.contains('cpu')",
92+
})
93+
);
94+
const { result } = renderHook(() =>
95+
useCelState({ enableQueryParams: false, defaultCel: "" })
96+
);
97+
98+
expect(result.current[0]).toBe("");
99+
});
100+
});
101+
102+
it("should remove cel query param when celState is reset to defaultCel", () => {
103+
(useSearchParams as jest.Mock).mockReturnValue(
104+
new URLSearchParams({
105+
cel: "name.contains('memory')",
106+
})
107+
);
108+
109+
const { result } = renderHook(() =>
110+
useCelState({
111+
enableQueryParams: true,
112+
defaultCel: "name.contains('cpu')",
113+
})
114+
);
115+
116+
act(() => {
117+
result.current[1]("name.contains('cpu')");
118+
});
119+
120+
expect(replaceMock).toHaveBeenCalledWith("/");
121+
});
122+
123+
it("should clean up cel query param when pathname changes", () => {
124+
(useSearchParams as jest.Mock).mockReturnValue(
125+
new URLSearchParams({
126+
cel: "name.contains('cpu')",
127+
})
128+
);
129+
130+
(usePathname as jest.Mock).mockReturnValue("/new/pathname");
131+
132+
const { result, unmount } = renderHook(() =>
133+
useCelState({ enableQueryParams: true, defaultCel: "" })
134+
);
135+
136+
unmount();
137+
138+
expect(replaceMock).toHaveBeenCalledWith("/");
139+
});
140+
});

‎keep-ui/features/cel-input/use-cel-state.ts

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,56 @@
1-
import { usePathname, useSearchParams } from "next/navigation";
2-
import { useEffect, useState } from "react";
1+
import { usePathname, useRouter, useSearchParams } from "next/navigation";
2+
import { useEffect, useRef, useState } from "react";
33
const celQueryParamName = "cel";
44
const defaultOptions = { enableQueryParams: false, defaultCel: "" };
55

66
export function useCelState({
77
enableQueryParams,
88
defaultCel,
99
}: typeof defaultOptions) {
10+
const router = useRouter();
1011
const pathname = usePathname();
1112
const searchParams = useSearchParams();
12-
const [celState, setCelState] = useState(
13-
() => searchParams.get(celQueryParamName) || defaultCel || ""
14-
);
13+
const searchParamsRef = useRef(searchParams);
14+
searchParamsRef.current = searchParams;
15+
const [celState, setCelState] = useState(() => {
16+
if (!enableQueryParams) {
17+
return defaultCel || "";
18+
}
19+
20+
return searchParams.get(celQueryParamName) || defaultCel || "";
21+
});
1522

1623
// Clean up cel param when pathname changes
1724
useEffect(() => {
1825
return () => {
19-
const newParams = new URLSearchParams(window.location.search);
26+
const newParams = new URLSearchParams(searchParamsRef.current);
2027
if (newParams.has(celQueryParamName)) {
2128
newParams.delete(celQueryParamName);
22-
window.history.replaceState(
23-
null,
24-
"",
25-
`${window.location.pathname}${newParams.toString() ? '?' + newParams.toString() : ''}`
29+
router.replace(
30+
`${window.location.pathname}${newParams.toString() ? "?" + newParams.toString() : ""}`
2631
);
2732
}
2833
};
2934
}, [pathname]);
3035

3136
useEffect(() => {
3237
if (!enableQueryParams) return;
38+
const paramsCopy = new URLSearchParams(searchParamsRef.current);
3339

34-
const params = new URLSearchParams(window.location.search);
35-
36-
if (params.get(celQueryParamName) === celState) {
40+
if (paramsCopy.get(celQueryParamName) === celState) {
3741
return;
3842
}
3943

40-
params.delete(celQueryParamName);
44+
paramsCopy.delete(celQueryParamName);
4145

4246
if (celState && celState !== defaultCel) {
43-
params.set(celQueryParamName, celState);
47+
paramsCopy.set(celQueryParamName, celState);
4448
}
4549

46-
window.history.replaceState(
47-
null,
48-
"",
49-
`${window.location.pathname}?${params}`
50+
router.replace(
51+
`${window.location.pathname}${paramsCopy.toString() ? "?" + paramsCopy.toString() : ""}`
5052
);
5153
}, [celState, enableQueryParams, defaultCel]);
5254

53-
5455
return [celState, setCelState] as const;
5556
}

‎keep-ui/features/presets/create-or-update-preset/ui/alerts-count-badge.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export const AlertsCountBadge: React.FC<AlertsCountBadgeProps> = ({
5858
vertical ? "flex-col" : "flex-row"
5959
} items-center gap-2`}
6060
>
61-
<Badge size="xl" color="orange">
61+
<Badge data-testid="alerts-count-badge" size="xl" color="orange">
6262
{totalCount}
6363
</Badge>
6464
<Text className="text-sm">

‎keep-ui/features/presets/create-or-update-preset/ui/create-or-update-preset-form.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,11 @@ export function CreateOrUpdatePresetForm({
153153
};
154154

155155
return (
156-
<form className="space-y-2" onSubmit={addOrUpdatePreset}>
156+
<form
157+
data-testid="preset-form"
158+
className="space-y-2"
159+
onSubmit={addOrUpdatePreset}
160+
>
157161
<div className="text-lg font-semibold">
158162
<p>{presetName ? "Update preset" : "Enter new preset name"}</p>
159163
</div>
@@ -163,6 +167,7 @@ export function CreateOrUpdatePresetForm({
163167
<div className="space-y-2">
164168
<div className="flex items-center gap-2">
165169
<TextInput
170+
data-testid="preset-name-input"
166171
// TODO: don't show error until user tries to save
167172
error={!presetName}
168173
errorMessage="Preset name is required"
@@ -253,6 +258,7 @@ export function CreateOrUpdatePresetForm({
253258
Close
254259
</Button>
255260
<Button
261+
data-testid="save-preset-button"
256262
disabled={!presetName}
257263
type="submit"
258264
size="lg"

‎keep-ui/features/presets/create-or-update-preset/ui/preset-controls.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ export const PresetControls: React.FC<PresetControlsProps> = ({
6161
</div>
6262
<div className="flex items-center gap-2">
6363
<Switch
64+
data-testid="counter-shows-firing-only-switch"
6465
id="counterShowsFiringOnly"
6566
checked={counterShowsFiringOnly}
6667
onChange={() => setCounterShowsFiringOnly(!counterShowsFiringOnly)}

‎keep-ui/features/presets/custom-preset-links/ui/CustomPresetAlertLinks.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ export const AlertPresetLink = ({
9292
isDeletable={isDeletable}
9393
onDelete={() => deletePreset && deletePreset(preset.id, preset.name)}
9494
isExact={true}
95+
testId="preset"
9596
renderBeforeCount={renderBeforeCount}
9697
className={clsx(
9798
"flex items-center space-x-2 p-1 text-slate-400 font-medium rounded-lg",

‎keep-ui/features/presets/presets-manager/ui/alerts-rules-builder.tsx

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,32 @@ const SQL_QUERY_PLACEHOLDER = `SELECT *
151151
FROM alerts
152152
WHERE severity = 'critical' and status = 'firing'`;
153153

154+
const constructCELRules = (preset?: Preset) => {
155+
// Check if selectedPreset is defined and has options
156+
if (preset && preset.options) {
157+
// New version: single "CEL" key
158+
const celOption = preset.options.find((option) => option.label === "CEL");
159+
if (celOption) {
160+
return celOption.value;
161+
}
162+
// Older version: Concatenate multiple fields
163+
else {
164+
return preset.options
165+
.map((option) => {
166+
// Assuming the older format is exactly "x='y'" (x equals y)
167+
// We split the string by '=', then trim and quote the value part
168+
let [key, value] = option.value.split("=");
169+
// Trim spaces and single quotes (if any) from the value
170+
value = value.trim().replace(/^'(.*)'$/, "$1");
171+
// Return the correctly formatted CEL expression
172+
return `${key.trim()}=="${value}"`;
173+
})
174+
.join(" && ");
175+
}
176+
}
177+
return ""; // Default to empty string if no preset or options are found
178+
};
179+
154180
export const AlertsRulesBuilder = ({
155181
table,
156182
selectedPreset,
@@ -178,13 +204,12 @@ export const AlertsRulesBuilder = ({
178204
const [isGUIOpen, setIsGUIOpen] = useState(false);
179205
const [isImportSQLOpen, setImportSQLOpen] = useState(false);
180206
const [sqlQuery, setSQLQuery] = useState("");
181-
const [celRules, setCELRules] = useState(
182-
searchParams?.get("cel") || defaultQuery
183-
);
207+
184208
const [appliedCel, setAppliedCel] = useCelState({
185-
enableQueryParams: true,
186-
defaultCel: defaultQuery,
209+
enableQueryParams: shouldSetQueryParam,
210+
defaultCel: constructCELRules(selectedPreset),
187211
});
212+
const [celRules, setCELRules] = useState(appliedCel);
188213

189214
const parsedCELRulesToQuery = parseCEL(celRules);
190215

@@ -221,32 +246,6 @@ export const AlertsRulesBuilder = ({
221246
toggleSuggestions();
222247
};
223248

224-
const constructCELRules = (preset?: Preset) => {
225-
// Check if selectedPreset is defined and has options
226-
if (preset && preset.options) {
227-
// New version: single "CEL" key
228-
const celOption = preset.options.find((option) => option.label === "CEL");
229-
if (celOption) {
230-
return celOption.value;
231-
}
232-
// Older version: Concatenate multiple fields
233-
else {
234-
return preset.options
235-
.map((option) => {
236-
// Assuming the older format is exactly "x='y'" (x equals y)
237-
// We split the string by '=', then trim and quote the value part
238-
let [key, value] = option.value.split("=");
239-
// Trim spaces and single quotes (if any) from the value
240-
value = value.trim().replace(/^'(.*)'$/, "$1");
241-
// Return the correctly formatted CEL expression
242-
return `${key.trim()}=="${value}"`;
243-
})
244-
.join(" && ");
245-
}
246-
}
247-
return ""; // Default to empty string if no preset or options are found
248-
};
249-
250249
useEffect(() => {
251250
function handleClickOutside(event: any) {
252251
if (wrapperRef.current && !wrapperRef.current.contains(event.target)) {
@@ -437,6 +436,7 @@ export const AlertsRulesBuilder = ({
437436
{/* Buttons next to the Textarea */}
438437
{showSave && (
439438
<Button
439+
data-testid="save-preset-button"
440440
icon={FiSave}
441441
color="orange"
442442
variant="secondary"

‎keep-ui/widgets/alerts-table/ui/alert-table-server-side.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,9 @@ export function AlertTableServerSide({
571571
<div className="flex flex-col gap-4">
572572
<div className="flex-none">
573573
<div className="flex justify-between">
574-
<PageTitle className="capitalize inline">{presetName}</PageTitle>
574+
<span data-testid="preset-page-title">
575+
<PageTitle className="capitalize inline">{presetName}</PageTitle>
576+
</span>
575577
<div className="grid grid-cols-[auto_auto] grid-rows-[auto_auto] gap-4">
576578
{timeFrame && (
577579
<EnhancedDateRangePickerV2

‎pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[tool.poetry]
22
name = "keep"
3-
version = "0.44.4"
3+
version = "0.44.5"
44
description = "Alerting. for developers, by developers."
55
authors = ["Keep Alerting LTD"]
66
packages = [{include = "keep"}]

0 commit comments

Comments
 (0)