Skip to content

Commit 5d82a78

Browse files
BrunoQuaresmaBrunoQuaresma
authored
fix(site): fix and improve pending state on template editor UI (#12766)
1 parent 47fd190 commit 5d82a78

File tree

6 files changed

+183
-119
lines changed

6 files changed

+183
-119
lines changed

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

+59-63
Original file line numberDiff line numberDiff line change
@@ -175,10 +175,10 @@ export const TemplateVersionEditor: FC<TemplateVersionEditorProps> = ({
175175
typeof editorValue === "string" ? isBinaryData(editorValue) : false;
176176

177177
// Auto scroll
178-
const buildLogsRef = useRef<HTMLDivElement>(null);
178+
const logsContentRef = useRef<HTMLDivElement>(null);
179179
useEffect(() => {
180-
if (buildLogsRef.current) {
181-
buildLogsRef.current.scrollTop = buildLogsRef.current.scrollHeight;
180+
if (logsContentRef.current) {
181+
logsContentRef.current.scrollTop = logsContentRef.current.scrollHeight;
182182
}
183183
}, [buildLogs]);
184184

@@ -237,9 +237,7 @@ export const TemplateVersionEditor: FC<TemplateVersionEditorProps> = ({
237237
paddingRight: 16,
238238
}}
239239
>
240-
{buildLogs && (
241-
<TemplateVersionStatusBadge version={templateVersion} />
242-
)}
240+
<TemplateVersionStatusBadge version={templateVersion} />
243241

244242
<ButtonGroup
245243
variant="outlined"
@@ -575,62 +573,51 @@ export const TemplateVersionEditor: FC<TemplateVersionEditorProps> = ({
575573
)}
576574
</div>
577575

578-
<div
579-
ref={buildLogsRef}
580-
css={{
581-
display: selectedTab !== "logs" ? "none" : "flex",
582-
height: selectedTab ? 280 : 0,
583-
flexDirection: "column",
584-
overflowY: "auto",
585-
}}
586-
>
587-
{templateVersion.job.error && (
588-
<div>
589-
<Alert
590-
severity="error"
591-
css={{
592-
borderRadius: 0,
593-
border: 0,
594-
borderBottom: `1px solid ${theme.palette.divider}`,
595-
borderLeft: `2px solid ${theme.palette.error.main}`,
596-
}}
597-
>
598-
<AlertTitle>Error during the build</AlertTitle>
599-
<AlertDetail>{templateVersion.job.error}</AlertDetail>
600-
</Alert>
601-
</div>
602-
)}
603-
604-
{buildLogs && buildLogs.length === 0 && (
605-
<Loader css={{ height: "100%" }} />
606-
)}
607-
608-
{buildLogs && buildLogs.length > 0 && (
609-
<WorkspaceBuildLogs
610-
css={styles.buildLogs}
611-
hideTimestamps
612-
logs={buildLogs}
613-
/>
614-
)}
615-
</div>
576+
{selectedTab === "logs" && (
577+
<div
578+
css={[styles.logs, styles.tabContent]}
579+
ref={logsContentRef}
580+
>
581+
{templateVersion.job.error && (
582+
<div>
583+
<Alert
584+
severity="error"
585+
css={{
586+
borderRadius: 0,
587+
border: 0,
588+
borderBottom: `1px solid ${theme.palette.divider}`,
589+
borderLeft: `2px solid ${theme.palette.error.main}`,
590+
}}
591+
>
592+
<AlertTitle>Error during the build</AlertTitle>
593+
<AlertDetail>{templateVersion.job.error}</AlertDetail>
594+
</Alert>
595+
</div>
596+
)}
597+
598+
{buildLogs && buildLogs.length > 0 ? (
599+
<WorkspaceBuildLogs
600+
css={styles.buildLogs}
601+
hideTimestamps
602+
logs={buildLogs}
603+
/>
604+
) : (
605+
<Loader css={{ height: "100%" }} />
606+
)}
607+
</div>
608+
)}
616609

617-
<div
618-
css={[
619-
{
620-
display: selectedTab !== "resources" ? "none" : undefined,
621-
height: selectedTab ? 280 : 0,
622-
},
623-
styles.resources,
624-
]}
625-
>
626-
{resources && (
627-
<TemplateResourcesTable
628-
resources={resources.filter(
629-
(r) => r.workspace_transition === "start",
630-
)}
631-
/>
632-
)}
633-
</div>
610+
{selectedTab === "resources" && (
611+
<div css={[styles.resources, styles.tabContent]}>
612+
{resources && (
613+
<TemplateResourcesTable
614+
resources={resources.filter(
615+
(r) => r.workspace_transition === "start",
616+
)}
617+
/>
618+
)}
619+
</div>
620+
)}
634621
</div>
635622
</div>
636623
</div>
@@ -751,6 +738,17 @@ const styles = {
751738
},
752739
}),
753740

741+
tabContent: {
742+
height: 280,
743+
overflowY: "auto",
744+
},
745+
746+
logs: {
747+
display: "flex",
748+
height: "100%",
749+
flexDirection: "column",
750+
},
751+
754752
buildLogs: {
755753
borderRadius: 0,
756754
border: 0,
@@ -780,8 +778,6 @@ const styles = {
780778
},
781779

782780
resources: {
783-
overflowY: "auto",
784-
785781
// Hack to access customize resource-card from here
786782
"& .resource-card": {
787783
borderLeft: 0,

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

+80-24
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { render, screen, waitFor, within } from "@testing-library/react";
22
import userEvent, { type UserEvent } from "@testing-library/user-event";
3+
import WS from "jest-websocket-mock";
34
import { HttpResponse, http } from "msw";
45
import { QueryClient } from "react-query";
56
import { RouterProvider, createMemoryRouter } from "react-router-dom";
@@ -16,6 +17,7 @@ import {
1617
MockWorkspaceBuildLogs,
1718
} from "testHelpers/entities";
1819
import {
20+
createTestQueryClient,
1921
renderWithAuth,
2022
waitForLoaderToBeRemoved,
2123
} from "testHelpers/renderHelpers";
@@ -291,30 +293,7 @@ describe.each([
291293
);
292294
}
293295

294-
render(
295-
<AppProviders queryClient={queryClient}>
296-
<RouterProvider
297-
router={createMemoryRouter(
298-
[
299-
{
300-
element: <RequireAuth />,
301-
children: [
302-
{
303-
element: <TemplateVersionEditorPage />,
304-
path: "/templates/:template/versions/:version/edit",
305-
},
306-
],
307-
},
308-
],
309-
{
310-
initialEntries: [
311-
`/templates/${MockTemplate.name}/versions/${MockTemplateVersion.name}/edit`,
312-
],
313-
},
314-
)}
315-
/>
316-
</AppProviders>,
317-
);
296+
renderEditorPage(queryClient);
318297
await waitForLoaderToBeRemoved();
319298

320299
const dialogSelector = /template variables/i;
@@ -326,3 +305,80 @@ describe.each([
326305
});
327306
},
328307
);
308+
309+
test("display pending badge and update it to running when status changes", async () => {
310+
const MockPendingTemplateVersion = {
311+
...MockTemplateVersion,
312+
job: {
313+
...MockTemplateVersion.job,
314+
status: "pending",
315+
},
316+
};
317+
const MockRunningTemplateVersion = {
318+
...MockTemplateVersion,
319+
job: {
320+
...MockTemplateVersion.job,
321+
status: "running",
322+
},
323+
};
324+
325+
let calls = 0;
326+
server.use(
327+
http.get(
328+
"/api/v2/organizations/:org/templates/:template/versions/:version",
329+
() => {
330+
calls += 1;
331+
return HttpResponse.json(
332+
calls > 1 ? MockRunningTemplateVersion : MockPendingTemplateVersion,
333+
);
334+
},
335+
),
336+
);
337+
338+
// Mock the logs when the status is running. This prevents connection errors
339+
// from being thrown in the console during the test.
340+
new WS(
341+
`ws://localhost/api/v2/templateversions/${MockTemplateVersion.name}/logs?follow=true`,
342+
);
343+
344+
renderEditorPage(createTestQueryClient());
345+
346+
const status = await screen.findByRole("status");
347+
expect(status).toHaveTextContent("Pending");
348+
349+
await waitFor(
350+
() => {
351+
expect(status).toHaveTextContent("Running");
352+
},
353+
// Increase the timeout due to the page fetching results every second, which
354+
// may cause delays.
355+
{ timeout: 5_000 },
356+
);
357+
});
358+
359+
function renderEditorPage(queryClient: QueryClient) {
360+
return render(
361+
<AppProviders queryClient={queryClient}>
362+
<RouterProvider
363+
router={createMemoryRouter(
364+
[
365+
{
366+
element: <RequireAuth />,
367+
children: [
368+
{
369+
element: <TemplateVersionEditorPage />,
370+
path: "/templates/:template/versions/:version/edit",
371+
},
372+
],
373+
},
374+
],
375+
{
376+
initialEntries: [
377+
`/templates/${MockTemplate.name}/versions/${MockTemplateVersion.name}/edit`,
378+
],
379+
},
380+
)}
381+
/>
382+
</AppProviders>,
383+
);
384+
}

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

+22-19
Original file line numberDiff line numberDiff line change
@@ -43,27 +43,31 @@ export const TemplateVersionEditorPage: FC = () => {
4343
templateName,
4444
versionName,
4545
);
46-
const templateVersionQuery = useQuery({
46+
const activeTemplateVersionQuery = useQuery({
4747
...templateVersionOptions,
4848
keepPreviousData: true,
49+
refetchInterval(data) {
50+
return data?.job.status === "pending" ? 1_000 : false;
51+
},
4952
});
53+
const { data: activeTemplateVersion } = activeTemplateVersionQuery;
5054
const uploadFileMutation = useMutation(uploadFile());
5155
const createTemplateVersionMutation = useMutation(
5256
createTemplateVersion(organizationId),
5357
);
5458
const resourcesQuery = useQuery({
55-
...resources(templateVersionQuery.data?.id ?? ""),
56-
enabled: templateVersionQuery.data?.job.status === "succeeded",
59+
...resources(activeTemplateVersion?.id ?? ""),
60+
enabled: activeTemplateVersion?.job.status === "succeeded",
5761
});
58-
const logs = useWatchVersionLogs(templateVersionQuery.data, {
59-
onDone: templateVersionQuery.refetch,
62+
const logs = useWatchVersionLogs(activeTemplateVersion, {
63+
onDone: activeTemplateVersionQuery.refetch,
6064
});
61-
const { fileTree, tarFile } = useFileTree(templateVersionQuery.data);
65+
const { fileTree, tarFile } = useFileTree(activeTemplateVersion);
6266
const {
6367
missingVariables,
6468
setIsMissingVariablesDialogOpen,
6569
isMissingVariablesDialogOpen,
66-
} = useMissingVariables(templateVersionQuery.data);
70+
} = useMissingVariables(activeTemplateVersion);
6771

6872
// Handle template publishing
6973
const [isPublishingDialogOpen, setIsPublishingDialogOpen] = useState(false);
@@ -109,25 +113,25 @@ export const TemplateVersionEditorPage: FC = () => {
109113
Record<string, string>
110114
>({});
111115
useEffect(() => {
112-
if (templateVersionQuery.data?.job.tags) {
113-
setProvisionerTags(templateVersionQuery.data.job.tags);
116+
if (activeTemplateVersion?.job.tags) {
117+
setProvisionerTags(activeTemplateVersion.job.tags);
114118
}
115-
}, [templateVersionQuery.data?.job.tags]);
119+
}, [activeTemplateVersion?.job.tags]);
116120

117121
return (
118122
<>
119123
<Helmet>
120124
<title>{pageTitle(`${templateName} · Template Editor`)}</title>
121125
</Helmet>
122126

123-
{!(templateQuery.data && templateVersionQuery.data && fileTree) ? (
127+
{!(templateQuery.data && activeTemplateVersion && fileTree) ? (
124128
<Loader fullscreen />
125129
) : (
126130
<TemplateVersionEditor
127131
activePath={activePath}
128132
onActivePathChange={onActivePathChange}
129133
template={templateQuery.data}
130-
templateVersion={templateVersionQuery.data}
134+
templateVersion={activeTemplateVersion}
131135
defaultFileTree={fileTree}
132136
onPreview={async (newFileTree) => {
133137
if (!tarFile) {
@@ -159,10 +163,10 @@ export const TemplateVersionEditorPage: FC = () => {
159163
await publishVersionMutation.mutateAsync({
160164
isActiveVersion,
161165
data,
162-
version: templateVersionQuery.data,
166+
version: activeTemplateVersion,
163167
});
164168
const publishedVersion = {
165-
...templateVersionQuery.data,
169+
...activeTemplateVersion,
166170
...data,
167171
};
168172
setIsPublishingDialogOpen(false);
@@ -190,13 +194,12 @@ export const TemplateVersionEditorPage: FC = () => {
190194
isBuilding={
191195
createTemplateVersionMutation.isLoading ||
192196
uploadFileMutation.isLoading ||
193-
templateVersionQuery.data.job.status === "running" ||
194-
templateVersionQuery.data.job.status === "pending"
197+
activeTemplateVersion.job.status === "running" ||
198+
activeTemplateVersion.job.status === "pending"
195199
}
196200
canPublish={
197-
templateVersionQuery.data.job.status === "succeeded" &&
198-
templateQuery.data.active_version_id !==
199-
templateVersionQuery.data.id
201+
activeTemplateVersion.job.status === "succeeded" &&
202+
templateQuery.data.active_version_id !== activeTemplateVersion.id
200203
}
201204
resources={resourcesQuery.data}
202205
buildLogs={logs}

0 commit comments

Comments
 (0)