Skip to content

Commit 4b27c77

Browse files
BrunoQuaresmaBrunoQuaresma
authored
fix(site): fix parameters' request upon template variables update (#11898)
Fix #11870
1 parent 60653bb commit 4b27c77

File tree

6 files changed

+149
-20
lines changed

6 files changed

+149
-20
lines changed

‎site/src/api/queries/templates.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,15 @@ export const templateVersions = (templateId: string) => {
127127
};
128128
};
129129

130+
export const templateVersionVariablesKey = (versionId: string) => [
131+
"templateVersion",
132+
versionId,
133+
"variables",
134+
];
135+
130136
export const templateVersionVariables = (versionId: string) => {
131137
return {
132-
queryKey: ["templateVersion", versionId, "variables"],
138+
queryKey: templateVersionVariablesKey(versionId),
133139
queryFn: () => API.getTemplateVersionVariables(versionId),
134140
};
135141
};

‎site/src/pages/TemplateVersionEditorPage/ProvisionerTagsPopover.tsx

+5-10
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
PopoverTrigger,
77
} from "components/Popover/Popover";
88
import { ProvisionerTag } from "pages/HealthPage/ProvisionerDaemonsPage";
9-
import { type FC } from "react";
9+
import { Fragment, type FC } from "react";
1010
import useTheme from "@mui/system/useTheme";
1111
import { useFormik } from "formik";
1212
import * as Yup from "yup";
@@ -111,18 +111,13 @@ export const ProvisionerTagsPopover: FC<ProvisionerTagsPopoverProps> = ({
111111
return key !== "owner";
112112
})
113113
.map((k) => (
114-
<>
114+
<Fragment key={k}>
115115
{k === "scope" ? (
116-
<ProvisionerTag key={k} k={k} v={tags[k]} />
116+
<ProvisionerTag k={k} v={tags[k]} />
117117
) : (
118-
<ProvisionerTag
119-
key={k}
120-
k={k}
121-
v={tags[k]}
122-
onDelete={onDelete}
123-
/>
118+
<ProvisionerTag k={k} v={tags[k]} onDelete={onDelete} />
124119
)}
125-
</>
120+
</Fragment>
126121
))}
127122
</Stack>
128123

‎site/src/pages/TemplateVersionEditorPage/TemplateVersionEditor.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ const styles = {
689689
padding: "0 16px",
690690
fontFamily: MONOSPACE_FONT_FAMILY,
691691

692-
"&:first-child": {
692+
"&:first-of-type": {
693693
paddingTop: 16,
694694
},
695695

@@ -715,7 +715,7 @@ const styles = {
715715
borderLeft: 0,
716716
borderRight: 0,
717717

718-
"&:first-child": {
718+
"&:first-of-type": {
719719
borderTop: 0,
720720
},
721721

‎site/src/pages/TemplateVersionEditorPage/TemplateVersionEditorPage.test.tsx

+122-2
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,26 @@
1-
import { renderWithAuth } from "testHelpers/renderHelpers";
1+
import {
2+
renderWithAuth,
3+
waitForLoaderToBeRemoved,
4+
} from "testHelpers/renderHelpers";
25
import TemplateVersionEditorPage from "./TemplateVersionEditorPage";
3-
import { screen, waitFor, within } from "@testing-library/react";
6+
import { render, screen, waitFor, within } from "@testing-library/react";
47
import userEvent from "@testing-library/user-event";
58
import * as api from "api/api";
69
import {
710
MockTemplate,
811
MockTemplateVersion,
12+
MockTemplateVersionVariable1,
13+
MockTemplateVersionVariable2,
914
MockWorkspaceBuildLogs,
1015
} from "testHelpers/entities";
1116
import { Language } from "./PublishTemplateVersionDialog";
17+
import { QueryClient } from "react-query";
18+
import { templateVersionVariablesKey } from "api/queries/templates";
19+
import { RouterProvider, createMemoryRouter } from "react-router-dom";
20+
import { RequireAuth } from "contexts/auth/RequireAuth";
21+
import { server } from "testHelpers/server";
22+
import { rest } from "msw";
23+
import { AppProviders } from "App";
1224

1325
// For some reason this component in Jest is throwing a MUI style warning so,
1426
// since we don't need it for this test, we can mock it out
@@ -208,3 +220,111 @@ test("Patch request is not send when there are no changes", async () => {
208220
);
209221
expect(patchTemplateVersion).toBeCalledTimes(0);
210222
});
223+
224+
describe.each([
225+
{
226+
testName: "Do not ask when template version has no errors",
227+
initialVariables: undefined,
228+
loadedVariables: undefined,
229+
templateVersion: MockTemplateVersion,
230+
askForVariables: false,
231+
},
232+
{
233+
testName:
234+
"Do not ask when template version has no errors even when having previously loaded variables",
235+
initialVariables: [
236+
MockTemplateVersionVariable1,
237+
MockTemplateVersionVariable2,
238+
],
239+
loadedVariables: undefined,
240+
templateVersion: MockTemplateVersion,
241+
askForVariables: false,
242+
},
243+
{
244+
testName: "Ask when template version has errors",
245+
initialVariables: undefined,
246+
templateVersion: {
247+
...MockTemplateVersion,
248+
job: {
249+
...MockTemplateVersion.job,
250+
error_code: "REQUIRED_TEMPLATE_VARIABLES",
251+
},
252+
},
253+
loadedVariables: [
254+
MockTemplateVersionVariable1,
255+
MockTemplateVersionVariable2,
256+
],
257+
askForVariables: true,
258+
},
259+
])(
260+
"Missing template variables",
261+
({
262+
testName,
263+
initialVariables,
264+
loadedVariables,
265+
templateVersion,
266+
askForVariables,
267+
}) => {
268+
it(testName, async () => {
269+
jest.resetAllMocks();
270+
const queryClient = new QueryClient();
271+
queryClient.setQueryData(
272+
templateVersionVariablesKey(MockTemplateVersion.id),
273+
initialVariables,
274+
);
275+
276+
server.use(
277+
rest.get(
278+
"/api/v2/organizations/:org/templates/:template/versions/:version",
279+
(req, res, ctx) => {
280+
return res(ctx.json(templateVersion));
281+
},
282+
),
283+
);
284+
285+
if (loadedVariables) {
286+
server.use(
287+
rest.get(
288+
"/api/v2/templateversions/:version/variables",
289+
(req, res, ctx) => {
290+
return res(ctx.json(loadedVariables));
291+
},
292+
),
293+
);
294+
}
295+
296+
render(
297+
<AppProviders queryClient={queryClient}>
298+
<RouterProvider
299+
router={createMemoryRouter(
300+
[
301+
{
302+
element: <RequireAuth />,
303+
children: [
304+
{
305+
element: <TemplateVersionEditorPage />,
306+
path: "/templates/:template/versions/:version/edit",
307+
},
308+
],
309+
},
310+
],
311+
{
312+
initialEntries: [
313+
`/templates/${MockTemplate.name}/versions/${MockTemplateVersion.name}/edit`,
314+
],
315+
},
316+
)}
317+
/>
318+
</AppProviders>,
319+
);
320+
await waitForLoaderToBeRemoved();
321+
322+
const dialogSelector = /template variables/i;
323+
if (askForVariables) {
324+
await screen.findByText(dialogSelector);
325+
} else {
326+
expect(screen.queryByText(dialogSelector)).not.toBeInTheDocument();
327+
}
328+
});
329+
},
330+
);

‎site/src/pages/TemplateVersionEditorPage/TemplateVersionEditorPage.tsx

+7-5
Original file line numberDiff line numberDiff line change
@@ -302,21 +302,23 @@ const useVersionLogs = (
302302
};
303303

304304
const useMissingVariables = (templateVersion: TemplateVersion | undefined) => {
305-
const { data: missingVariables } = useQuery({
305+
const isRequiringVariables =
306+
templateVersion?.job.error_code === "REQUIRED_TEMPLATE_VARIABLES";
307+
const { data: variables } = useQuery({
306308
...templateVersionVariables(templateVersion?.id ?? ""),
307-
enabled: templateVersion?.job.error_code === "REQUIRED_TEMPLATE_VARIABLES",
309+
enabled: isRequiringVariables,
308310
});
309311
const [isMissingVariablesDialogOpen, setIsMissingVariablesDialogOpen] =
310312
useState(false);
311313

312314
useEffect(() => {
313-
if (missingVariables) {
315+
if (isRequiringVariables) {
314316
setIsMissingVariablesDialogOpen(true);
315317
}
316-
}, [missingVariables]);
318+
}, [isRequiringVariables]);
317319

318320
return {
319-
missingVariables,
321+
missingVariables: isRequiringVariables ? variables : undefined,
320322
isMissingVariablesDialogOpen,
321323
setIsMissingVariablesDialogOpen,
322324
};

‎site/src/testHelpers/handlers.ts

+6
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,12 @@ export const handlers = [
8282
ctx.json([M.MockTemplateVersion2, M.MockTemplateVersion]),
8383
);
8484
}),
85+
rest.patch(
86+
"/api/v2/templates/:templateId/versions",
87+
async (req, res, ctx) => {
88+
return res(ctx.status(200), ctx.json({}));
89+
},
90+
),
8591
rest.patch("/api/v2/templates/:templateId", async (req, res, ctx) => {
8692
return res(ctx.status(200), ctx.json(M.MockTemplate));
8793
}),

0 commit comments

Comments
 (0)