Remove form load from service

coder · Parkreiner · Oct 11, 2023 · Oct 10, 2023 · Oct 10, 2023 · Oct 10, 2023
commit 816e596f2a51d54d2a9510363ed61f14e71760c8
diff --git a/site/src/App.tsx b/site/src/App.tsx
@@ -14,7 +14,7 @@ import {
 } from "@mui/material/styles";
 import { ThemeProvider as EmotionThemeProvider } from "@emotion/react";
 
-const queryClient = new QueryClient({
+const defaultQueryClient = new QueryClient({
   defaultOptions: {
     queries: {
       retry: false,
@@ -38,7 +38,9 @@ export const ThemeProviders: FC<PropsWithChildren> = ({ children }) => {
   );
 };
 
-export const AppProviders: FC<PropsWithChildren> = ({ children }) => {
+export const AppProviders: FC<
+  PropsWithChildren<{ queryClient?: QueryClient }>
+> = ({ children, queryClient = defaultQueryClient }) => {
   return (
     <HelmetProvider>
       <ThemeProviders>

@@ -6,3 +6,10 @@ export const templateVersionLogs = (versionId: string) => {
     queryFn: () => API.getTemplateVersionLogs(versionId),
   };
 };
+
+export const richParameters = (versionId: string) => {
+  return {
+    queryKey: ["templateVersion", versionId, "richParameters"],
+    queryFn: () => API.getTemplateVersionRichParameters(versionId),
+  };
+};
@@ -9,10 +9,7 @@ import { type FC, useCallback, useState, useEffect } from "react";
 import { Helmet } from "react-helmet-async";
 import { useNavigate, useParams, useSearchParams } from "react-router-dom";
 import { pageTitle } from "utils/page";
-import {
-  CreateWSPermissions,
-  createWorkspaceMachine,
-} from "xServices/createWorkspace/createWorkspaceXService";
+import { createWorkspaceMachine } from "xServices/createWorkspace/createWorkspaceXService";
 import { CreateWorkspacePageView } from "./CreateWorkspacePageView";
 import { Loader } from "components/Loader/Loader";
 import { ErrorAlert } from "components/Alert/ErrorAlert";
@@ -23,8 +20,15 @@ import {
   NumberDictionary,
 } from "unique-names-generator";
 import { useMutation, useQuery, useQueryClient } from "react-query";
-import { templateVersionExternalAuth } from "api/queries/templates";
+import {
+  templateByName,
+  templateVersionExternalAuth,
+} from "api/queries/templates";
 import { autoCreateWorkspace } from "api/queries/workspaces";
+import { checkAuthorization } from "api/queries/authCheck";
+import { CreateWSPermissions, createWorkspaceChecks } from "./permissions";
+import { richParameters } from "api/queries/templateVersions";
+import { paramsUsedToCreateWorkspace } from "utils/workspace";
 
 type CreateWorkspaceMode = "form" | "auto";
 
@@ -69,6 +73,21 @@ const CreateWorkspacePage: FC = () => {
     autoCreateWorkspace(queryClient),
   );
 
+  const templateQuery = useQuery(templateByName(organizationId, templateName));
+  const permissionsQuery = useQuery(
+    checkAuthorization({
+      checks: createWorkspaceChecks(organizationId),
+    }),
+  );
+  const realizedVersionId = versionId ?? templateQuery.data?.active_version_id;
+  const richParametersQuery = useQuery({
+    ...richParameters(realizedVersionId ?? ""),
+    enabled: realizedVersionId !== undefined,
+  });
+  const realizedParameters = richParametersQuery.data
+    ? richParametersQuery.data.filter(paramsUsedToCreateWorkspace)
+    : undefined;
+
   const title = autoCreateWorkspaceMutation.isLoading
     ? "Creating workspace..."
     : "Create workspace";
@@ -105,6 +124,31 @@ const CreateWorkspacePage: FC = () => {
     templateName,
   ]);
 
+  useEffect(() => {
+    if (
+      templateQuery.data &&
+      permissionsQuery.data &&
+      realizedParameters &&
+      realizedVersionId
+    ) {
+      send({
+        type: "LOAD_FORM_DATA",
+        data: {
+          template: templateQuery.data,
+          permissions: permissionsQuery.data as CreateWSPermissions,
+          parameters: realizedParameters,
+          versionId: realizedVersionId,
+        },
+      });
+    }
+  }, [
+    permissionsQuery.data,
+    realizedParameters,
+    send,
+    templateQuery.data,
+    realizedVersionId,
+  ]);
+
   return (
     <>
       <Helmet>

@@ -26,12 +26,12 @@ import {
   ImmutableTemplateParametersSection,
   MutableTemplateParametersSection,
 } from "components/TemplateParameters/TemplateParameters";
-import { CreateWSPermissions } from "xServices/createWorkspace/createWorkspaceXService";
 import { ExternalAuth } from "./ExternalAuth";
 import { ErrorAlert } from "components/Alert/ErrorAlert";
 import { Stack } from "components/Stack/Stack";
 import { type ExternalAuthPollingState } from "./CreateWorkspacePage";
 import { useSearchParams } from "react-router-dom";
+import { CreateWSPermissions } from "./permissions";
 
 export interface CreateWorkspacePageViewProps {
   error: unknown;

@@ -0,0 +1,16 @@
+export const createWorkspaceChecks = (organizationId: string) =>
+  ({
+    createWorkspaceForUser: {
+      object: {
+        resource_type: "workspace",
+        organization_id: organizationId,
+        owner_id: "*",
+      },
+      action: "create",
+    },
+  }) as const;
+
+export type CreateWSPermissions = Record<
+  keyof ReturnType<typeof createWorkspaceChecks>,
+  boolean
+>;
diff --git a/site/src/testHelpers/renderHelpers.tsx b/site/src/testHelpers/renderHelpers.tsx
@@ -11,13 +11,29 @@ import {
 import { RequireAuth } from "../components/RequireAuth/RequireAuth";
 import { MockUser } from "./entities";
 import { ReactNode } from "react";
+import { QueryClient } from "react-query";
 
 export const renderWithRouter = (
   router: ReturnType<typeof createMemoryRouter>,
 ) => {
   return {
     ...tlRender(
-      <AppProviders>
+      <AppProviders
+        queryClient={
+          // Create one query client for each render isolate it avoid other
+          // tests to be affected
+          new QueryClient({
+            defaultOptions: {
+              queries: {
+                retry: false,
+                cacheTime: 0,
+                refetchOnWindowFocus: false,
+                networkMode: "offlineFirst",
+              },
+            },
+          })
+        }
+      >
         <RouterProvider router={router} />
       </AppProviders>,
     ),

@@ -1,9 +1,4 @@
-import {
-  checkAuthorization,
-  createWorkspace,
-  getTemplateByName,
-  getTemplateVersionRichParameters,
-} from "api/api";
+import { createWorkspace } from "api/api";
 import {
   CreateWorkspaceRequest,
   Template,
@@ -12,8 +7,8 @@ import {
   Workspace,
   WorkspaceBuildParameter,
 } from "api/typesGenerated";
+import { CreateWSPermissions } from "pages/CreateWorkspacePage/permissions";
 import { assign, createMachine } from "xstate";
-import { paramsUsedToCreateWorkspace } from "utils/workspace";
 
 type CreateWorkspaceContext = {
   organizationId: string;
@@ -46,15 +41,18 @@ export const createWorkspaceMachine =
       tsTypes: {} as import("./createWorkspaceXService.typegen").Typegen0,
       schema: {
         context: {} as CreateWorkspaceContext,
-        events: {} as CreateWorkspaceEvent,
+        events: {} as
+          | CreateWorkspaceEvent
+          | {
+              type: "LOAD_FORM_DATA";
+              data: {
+                template: Template;
+                permissions: CreateWSPermissions;
+                parameters: TemplateVersionParameter[];
+                versionId: string;
+              };
+            },
         services: {} as {
-          loadFormData: {
-            data: {
-              template: Template;
-              permissions: CreateWSPermissions;
-              parameters: TemplateVersionParameter[];
-            };
-          };
           createWorkspace: {
             data: Workspace;
           };
@@ -63,16 +61,11 @@ export const createWorkspaceMachine =
       initial: "loadingFormData",
       states: {
         loadingFormData: {
-          invoke: {
-            src: "loadFormData",
-            onDone: {
+          on: {
+            LOAD_FORM_DATA: {
               target: "idle",
               actions: ["assignFormData"],
             },
-            onError: {
-              target: "loadError",
-              actions: ["assignError"],
-            },
           },
         },
         idle: {
@@ -120,26 +113,6 @@ export const createWorkspaceMachine =
 
           return createWorkspace(organizationId, owner.id, request);
         },
-        loadFormData: async ({ templateName, organizationId, versionId }) => {
-          const [template, permissions] = await Promise.all([
-            getTemplateByName(organizationId, templateName),
-            checkCreateWSPermissions(organizationId),
-          ]);
-
-          const realizedVersionId = versionId ?? template.active_version_id;
-          const [parameters] = await Promise.all([
-            getTemplateVersionRichParameters(realizedVersionId).then((p) =>
-              p.filter(paramsUsedToCreateWorkspace),
-            ),
-          ]);
-
-          return {
-            template,
-            permissions,
-            parameters,
-            versionId: realizedVersionId,
-          };
-        },
       },
       actions: {
         assignFormData: assign((ctx, event) => {
@@ -157,27 +130,3 @@ export const createWorkspaceMachine =
       },
     },
   );
-
-const checkCreateWSPermissions = async (organizationId: string) => {
-  // HACK: below, we pass in * for the owner_id, which is a hacky way of checking if the
-  // current user can create a workspace on behalf of anyone within the org (only org owners should be able to do this).
-  // This pattern should not be replicated outside of this narrow use case.
-  const permissionsToCheck = {
-    createWorkspaceForUser: {
-      object: {
-        resource_type: "workspace",
-        organization_id: organizationId,
-        owner_id: "*",
-      },
-      action: "create",
-    },
-  } as const;
-
-  return checkAuthorization({
-    checks: permissionsToCheck,
-  }) as Promise<Record<keyof typeof permissionsToCheck, boolean>>;
-};
-
-export type CreateWSPermissions = Awaited<
-  ReturnType<typeof checkCreateWSPermissions>
->;