Skip to content

Commit e16f3d0

Browse files
ericharelucaseduoli
authored andcommitted
fix: ensure global vars load properly on first flow (#12538)
* fix: Ensure global vars load properly on first flow * Add tests --------- Co-authored-by: Lucas Oliveira <62335616+lucaseduoli@users.noreply.github.com>
1 parent c6c99f8 commit e16f3d0

3 files changed

Lines changed: 158 additions & 9 deletions

File tree

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import { render, waitFor } from "@testing-library/react";
2+
import InputGlobalComponent from "..";
3+
4+
const mockUseGetGlobalVariables = jest.fn();
5+
6+
jest.mock("@/controllers/API/queries/variables", () => ({
7+
useGetGlobalVariables: () => mockUseGetGlobalVariables(),
8+
}));
9+
10+
jest.mock("@/shared/components/delete-confirmation-modal", () => () => null);
11+
12+
jest.mock(
13+
"@/components/core/GlobalVariableModal/GlobalVariableModal",
14+
() =>
15+
function GlobalVariableModal({ children }: { children?: React.ReactNode }) {
16+
return <>{children}</>;
17+
},
18+
);
19+
20+
jest.mock("@/components/common/genericIconComponent", () => ({
21+
__esModule: true,
22+
default: () => null,
23+
}));
24+
25+
jest.mock("@/components/ui/command", () => ({
26+
CommandItem: ({ children }: { children?: React.ReactNode }) => (
27+
<div>{children}</div>
28+
),
29+
}));
30+
31+
jest.mock(
32+
"@/components/core/parameterRenderComponent/components/inputComponent",
33+
() => ({
34+
__esModule: true,
35+
default: () => null,
36+
}),
37+
);
38+
39+
describe("InputGlobalComponent", () => {
40+
const handleOnNewValue = jest.fn();
41+
42+
const renderComponent = () =>
43+
render(
44+
<InputGlobalComponent
45+
id="global-var-input"
46+
value="MISSING_VAR"
47+
display_name="API Key"
48+
handleOnNewValue={handleOnNewValue}
49+
load_from_db
50+
password={false}
51+
editNode={false}
52+
disabled={false}
53+
/>,
54+
);
55+
56+
beforeEach(() => {
57+
jest.clearAllMocks();
58+
});
59+
60+
it("clears missing saved variables only after a successful settled fetch", async () => {
61+
mockUseGetGlobalVariables.mockReturnValue({
62+
data: [],
63+
isFetchedAfterMount: true,
64+
isFetching: false,
65+
isSuccess: true,
66+
});
67+
68+
renderComponent();
69+
70+
await waitFor(() => {
71+
expect(handleOnNewValue).toHaveBeenCalledWith(
72+
{ value: "", load_from_db: false },
73+
{ skipSnapshot: true },
74+
);
75+
});
76+
});
77+
78+
it("does not clear while a background refetch is still in flight", async () => {
79+
mockUseGetGlobalVariables.mockReturnValue({
80+
data: [{ name: "OTHER_VAR" }],
81+
isFetchedAfterMount: false,
82+
isFetching: true,
83+
isSuccess: true,
84+
});
85+
86+
renderComponent();
87+
88+
await waitFor(() => {
89+
expect(handleOnNewValue).not.toHaveBeenCalled();
90+
});
91+
});
92+
93+
it("does not clear when the global variables query fails", async () => {
94+
mockUseGetGlobalVariables.mockReturnValue({
95+
data: undefined,
96+
isFetchedAfterMount: true,
97+
isFetching: false,
98+
isSuccess: false,
99+
});
100+
101+
renderComponent();
102+
103+
await waitFor(() => {
104+
expect(handleOnNewValue).not.toHaveBeenCalled();
105+
});
106+
});
107+
});

src/frontend/src/components/core/parameterRenderComponent/components/inputGlobalComponent/hooks.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useEffect, useMemo, useRef } from "react";
1+
import { useEffect, useMemo, useRef } from "react";
22
import { useGlobalVariablesStore } from "@/stores/globalVariablesStore/globalVariables";
33
import type { GlobalVariable } from "./types";
44

@@ -41,6 +41,7 @@ export const useInitialLoad = (
4141
disabled: boolean,
4242
loadFromDb: boolean,
4343
globalVariables: GlobalVariable[],
44+
canValidateMissingVariable: boolean,
4445
valueExists: boolean,
4546
unavailableField: string | null,
4647
handleOnNewValue: (
@@ -54,17 +55,32 @@ export const useInitialLoad = (
5455
// Keep the latest handleOnNewValue reference
5556
handleOnNewValueRef.current = handleOnNewValue;
5657

57-
// Handle database loading when value doesn't exist
58+
// Handle database loading when value doesn't exist.
59+
// Guard on the settled query state so we don't clear values while the
60+
// global variables query is still in flight, during background refetches,
61+
// or after failed fetches.
5862
useEffect(() => {
59-
if (disabled || !loadFromDb || !globalVariables.length || valueExists) {
63+
if (
64+
disabled ||
65+
!loadFromDb ||
66+
!canValidateMissingVariable ||
67+
!globalVariables.length ||
68+
valueExists
69+
) {
6070
return;
6171
}
6272

6373
handleOnNewValueRef.current(
6474
{ value: "", load_from_db: false },
6575
{ skipSnapshot: true },
6676
);
67-
}, [disabled, loadFromDb, globalVariables.length, valueExists]);
77+
}, [
78+
disabled,
79+
loadFromDb,
80+
canValidateMissingVariable,
81+
globalVariables.length,
82+
valueExists,
83+
]);
6884

6985
// Handle unavailable field initialization
7086
useEffect(() => {

src/frontend/src/components/core/parameterRenderComponent/components/inputGlobalComponent/index.tsx

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import { useEffect } from "react";
22
import { useGetGlobalVariables } from "@/controllers/API/queries/variables";
33
import GeneralDeleteConfirmationModal from "@/shared/components/delete-confirmation-modal";
4+
import { looksLikeVariableName } from "../../../../../utils/reactflowUtils";
45
import { cn } from "../../../../../utils/utils";
56
import ForwardedIconComponent from "../../../../common/genericIconComponent";
67
import { CommandItem } from "../../../../ui/command";
78
import GlobalVariableModal from "../../../GlobalVariableModal/GlobalVariableModal";
89
import { getPlaceholder } from "../../helpers/get-placeholder-disabled";
910
import type { InputGlobalComponentType, InputProps } from "../../types";
10-
import { looksLikeVariableName } from "../../../../../utils/reactflowUtils";
1111
import InputComponent from "../inputComponent";
1212
import {
1313
useGlobalVariableValue,
@@ -30,7 +30,12 @@ export default function InputGlobalComponent({
3030
hasRefreshButton = false,
3131
showParameter = true,
3232
}: InputProps<string, InputGlobalComponentType>): JSX.Element | null {
33-
const { data: globalVariables } = useGetGlobalVariables();
33+
const {
34+
data: globalVariables,
35+
isFetchedAfterMount: isGlobalVariablesFetchedAfterMount,
36+
isFetching: isGlobalVariablesFetching,
37+
isSuccess: isGlobalVariablesFetchSuccessful,
38+
} = useGetGlobalVariables();
3439

3540
// // Safely cast the data to our typed interface
3641
const typedGlobalVariables: GlobalVariable[] = globalVariables ?? [];
@@ -44,25 +49,46 @@ export default function InputGlobalComponent({
4449
typedGlobalVariables,
4550
);
4651
const unavailableField = useUnavailableField(display_name, currentValue);
52+
const canValidateMissingVariable =
53+
isGlobalVariablesFetchSuccessful &&
54+
!isGlobalVariablesFetching &&
55+
isGlobalVariablesFetchedAfterMount;
4756

4857
useInitialLoad(
4958
isDisabled,
5059
loadFromDb,
5160
typedGlobalVariables,
61+
canValidateMissingVariable,
5262
valueExists,
5363
unavailableField,
5464
handleOnNewValue,
5565
);
5666

57-
// Clean up when selected variable no longer exists
67+
// Clean up when selected variable no longer exists.
68+
// Only validate against a successful, settled query result for this mount.
69+
// This avoids clearing values during the initial fetch, during background
70+
// refetches against cached data, or after failed requests.
5871
useEffect(() => {
59-
if (loadFromDb && currentValue && !valueExists && !isDisabled) {
72+
if (
73+
canValidateMissingVariable &&
74+
loadFromDb &&
75+
currentValue &&
76+
!valueExists &&
77+
!isDisabled
78+
) {
6079
handleOnNewValue(
6180
{ value: "", load_from_db: false },
6281
{ skipSnapshot: true },
6382
);
6483
}
65-
}, [loadFromDb, currentValue, valueExists, isDisabled, handleOnNewValue]);
84+
}, [
85+
canValidateMissingVariable,
86+
loadFromDb,
87+
currentValue,
88+
valueExists,
89+
isDisabled,
90+
handleOnNewValue,
91+
]);
6692

6793
// Create handlers object for better organization
6894
const handlers: GlobalVariableHandlers = {

0 commit comments

Comments
 (0)