refactor(site): refactor external auth component (#11758)

Recommended improvements:
- Rename component for clarity 
- Simplify interface for contextual relevance 
- Handle polling errors based on section, not every button

Before:
<img width="1511" alt="Screenshot 2024-01-22 at 15 24 26" src="https://github.com/coder/coder/assets/3165839/cfb8c0bc-f5a2-4708-bd97-fdfc46bd1eee">

Now:
<img width="1512" alt="Screenshot 2024-01-22 at 15 24 41" src="https://github.com/coder/coder/assets/3165839/5aaad448-1bb2-45ea-9250-cd374a072be2">
This commit is contained in:
Bruno Quaresma
2024-01-23 12:26:12 -03:00
committed by GitHub
parent 059e533544
commit 910f17f4e7
7 changed files with 213 additions and 254 deletions
@@ -201,7 +201,7 @@ describe("CreateWorkspacePage", () => {
); );
}); });
it("external auth: errors if unauthenticated and submits", async () => { it("external auth: errors if unauthenticated", async () => {
jest jest
.spyOn(API, "getTemplateVersionExternalAuth") .spyOn(API, "getTemplateVersionExternalAuth")
.mockResolvedValueOnce([MockTemplateVersionExternalAuthGithub]); .mockResolvedValueOnce([MockTemplateVersionExternalAuthGithub]);
@@ -209,17 +209,9 @@ describe("CreateWorkspacePage", () => {
renderCreateWorkspacePage(); renderCreateWorkspacePage();
await waitForLoaderToBeRemoved(); await waitForLoaderToBeRemoved();
const nameField = await screen.findByLabelText(nameLabelText); await screen.findByText(
"To create a workspace using the selected template, please ensure you are authenticated with all the external providers listed below.",
// have to use fireEvent b/c userEvent isn't cleaning up properly between tests );
fireEvent.change(nameField, {
target: { value: "test" },
});
const submitButton = screen.getByText(createWorkspaceText);
await userEvent.click(submitButton);
await screen.findByText("You must authenticate to create a workspace!");
}); });
it("auto create a workspace if uses mode=auto", async () => { it("auto create a workspace if uses mode=auto", async () => {
@@ -68,8 +68,12 @@ const CreateWorkspacePage: FC = () => {
? richParametersQuery.data.filter(paramsUsedToCreateWorkspace) ? richParametersQuery.data.filter(paramsUsedToCreateWorkspace)
: undefined; : undefined;
const { externalAuth, externalAuthPollingState, startPollingExternalAuth } = const {
useExternalAuth(realizedVersionId); externalAuth,
externalAuthPollingState,
startPollingExternalAuth,
isLoadingExternalAuth,
} = useExternalAuth(realizedVersionId);
const isLoadingFormData = const isLoadingFormData =
templateQuery.isLoading || templateQuery.isLoading ||
@@ -118,7 +122,9 @@ const CreateWorkspacePage: FC = () => {
<title>{pageTitle(title)}</title> <title>{pageTitle(title)}</title>
</Helmet> </Helmet>
{loadFormDataError && <ErrorAlert error={loadFormDataError} />} {loadFormDataError && <ErrorAlert error={loadFormDataError} />}
{isLoadingFormData || autoCreateWorkspaceMutation.isLoading ? ( {isLoadingFormData ||
isLoadingExternalAuth ||
autoCreateWorkspaceMutation.isLoading ? (
<Loader /> <Loader />
) : ( ) : (
<CreateWorkspacePageView <CreateWorkspacePageView
@@ -169,7 +175,7 @@ const useExternalAuth = (versionId: string | undefined) => {
setExternalAuthPollingState("polling"); setExternalAuthPollingState("polling");
}, []); }, []);
const { data: externalAuth } = useQuery( const { data: externalAuth, isLoading: isLoadingExternalAuth } = useQuery(
versionId versionId
? { ? {
...templateVersionExternalAuth(versionId), ...templateVersionExternalAuth(versionId),
@@ -205,6 +211,7 @@ const useExternalAuth = (versionId: string | undefined) => {
startPollingExternalAuth, startPollingExternalAuth,
externalAuth, externalAuth,
externalAuthPollingState, externalAuthPollingState,
isLoadingExternalAuth,
}; };
}; };
@@ -27,12 +27,12 @@ import {
ImmutableTemplateParametersSection, ImmutableTemplateParametersSection,
MutableTemplateParametersSection, MutableTemplateParametersSection,
} from "components/TemplateParameters/TemplateParameters"; } from "components/TemplateParameters/TemplateParameters";
import { ExternalAuth } from "./ExternalAuth"; import { ExternalAuthButton } from "./ExternalAuthButton";
import { ErrorAlert } from "components/Alert/ErrorAlert"; import { ErrorAlert } from "components/Alert/ErrorAlert";
import { Stack } from "components/Stack/Stack"; import { Stack } from "components/Stack/Stack";
import { import {
CreateWorkspaceMode, CreateWorkspaceMode,
type ExternalAuthPollingState, ExternalAuthPollingState,
} from "./CreateWorkspacePage"; } from "./CreateWorkspacePage";
import { useSearchParams } from "react-router-dom"; import { useSearchParams } from "react-router-dom";
import { CreateWSPermissions } from "./permissions"; import { CreateWSPermissions } from "./permissions";
@@ -85,10 +85,9 @@ export const CreateWorkspacePageView: FC<CreateWorkspacePageViewProps> = ({
}) => { }) => {
const theme = useTheme(); const theme = useTheme();
const [owner, setOwner] = useState(defaultOwner); const [owner, setOwner] = useState(defaultOwner);
const { verifyExternalAuth, externalAuthErrors } =
useExternalAuthVerification(externalAuth);
const [searchParams] = useSearchParams(); const [searchParams] = useSearchParams();
const disabledParamsList = searchParams?.get("disable_params")?.split(","); const disabledParamsList = searchParams?.get("disable_params")?.split(",");
const requiresExternalAuth = externalAuth.some((auth) => !auth.authenticated);
const form: FormikContextType<TypesGen.CreateWorkspaceRequest> = const form: FormikContextType<TypesGen.CreateWorkspaceRequest> =
useFormik<TypesGen.CreateWorkspaceRequest>({ useFormik<TypesGen.CreateWorkspaceRequest>({
@@ -106,7 +105,7 @@ export const CreateWorkspacePageView: FC<CreateWorkspacePageViewProps> = ({
}), }),
enableReinitialize: true, enableReinitialize: true,
onSubmit: (request) => { onSubmit: (request) => {
if (!verifyExternalAuth()) { if (requiresExternalAuth) {
return; return;
} }
@@ -192,16 +191,20 @@ export const CreateWorkspacePageView: FC<CreateWorkspacePageViewProps> = ({
description="This template requires authentication to external services." description="This template requires authentication to external services."
> >
<FormFields> <FormFields>
{requiresExternalAuth && (
<Alert severity="error">
To create a workspace using the selected template, please
ensure you are authenticated with all the external providers
listed below.
</Alert>
)}
{externalAuth.map((auth) => ( {externalAuth.map((auth) => (
<ExternalAuth <ExternalAuthButton
key={auth.id} key={auth.id}
authenticateURL={auth.authenticate_url} auth={auth}
authenticated={auth.authenticated} isLoading={externalAuthPollingState === "polling"}
externalAuthPollingState={externalAuthPollingState} onStartPolling={startPollingExternalAuth}
startPollingExternalAuth={startPollingExternalAuth} displayRetry={externalAuthPollingState === "abandoned"}
displayName={auth.display_name}
displayIcon={auth.display_icon}
error={externalAuthErrors[auth.id]}
/> />
))} ))}
</FormFields> </FormFields>
@@ -273,43 +276,6 @@ export const CreateWorkspacePageView: FC<CreateWorkspacePageViewProps> = ({
); );
}; };
type ExternalAuthErrors = Record<string, string>;
const useExternalAuthVerification = (
externalAuth: TypesGen.TemplateVersionExternalAuth[],
) => {
const [externalAuthErrors, setExternalAuthErrors] =
useState<ExternalAuthErrors>({});
// Clear errors when externalAuth is refreshed
useEffect(() => {
setExternalAuthErrors({});
}, [externalAuth]);
const verifyExternalAuth = () => {
const errors: ExternalAuthErrors = {};
for (let i = 0; i < externalAuth.length; i++) {
const auth = externalAuth.at(i);
if (!auth) {
continue;
}
if (!auth.authenticated) {
errors[auth.id] = "You must authenticate to create a workspace!";
}
}
setExternalAuthErrors(errors);
const isValid = Object.keys(errors).length === 0;
return isValid;
};
return {
externalAuthErrors,
verifyExternalAuth,
};
};
const styles = { const styles = {
hasDescription: { hasDescription: {
paddingBottom: 16, paddingBottom: 16,
@@ -1,92 +0,0 @@
import { ExternalAuth } from "./ExternalAuth";
import type { Meta, StoryObj } from "@storybook/react";
const meta: Meta<typeof ExternalAuth> = {
title: "pages/CreateWorkspacePage/ExternalAuth",
component: ExternalAuth,
};
export default meta;
type Story = StoryObj<typeof ExternalAuth>;
export const Github: Story = {
args: {
displayIcon: "/icon/github.svg",
displayName: "GitHub",
authenticated: false,
},
};
export const GithubTimeout: Story = {
args: {
displayIcon: "/icon/github.svg",
displayName: "GitHub",
authenticated: false,
externalAuthPollingState: "abandoned",
},
};
export const GithubFailed: Story = {
args: {
displayIcon: "/icon/github.svg",
displayName: "GitHub",
authenticated: false,
error: "Github doesn't like you",
},
};
export const GithubAuthenticated: Story = {
args: {
displayIcon: "/icon/github.svg",
displayName: "GitHub",
authenticated: true,
},
};
export const Gitlab: Story = {
args: {
displayIcon: "/icon/gitlab.svg",
displayName: "GitLab",
authenticated: false,
},
};
export const GitlabAuthenticated: Story = {
args: {
displayIcon: "/icon/gitlab.svg",
displayName: "GitLab",
authenticated: true,
},
};
export const AzureDevOps: Story = {
args: {
displayIcon: "/icon/azure-devops.svg",
displayName: "Azure DevOps",
authenticated: false,
},
};
export const AzureDevOpsAuthenticated: Story = {
args: {
displayIcon: "/icon/azure-devops.svg",
displayName: "Azure DevOps",
authenticated: true,
},
};
export const Bitbucket: Story = {
args: {
displayIcon: "/icon/bitbucket.svg",
displayName: "Bitbucket",
authenticated: false,
},
};
export const BitbucketAuthenticated: Story = {
args: {
displayIcon: "/icon/bitbucket.svg",
displayName: "Bitbucket",
authenticated: true,
},
};
@@ -1,96 +0,0 @@
import ReplayIcon from "@mui/icons-material/Replay";
import Button from "@mui/material/Button";
import FormHelperText from "@mui/material/FormHelperText";
import Tooltip from "@mui/material/Tooltip";
import { type FC } from "react";
import { Stack } from "components/Stack/Stack";
import { type ExternalAuthPollingState } from "./CreateWorkspacePage";
import LoadingButton from "@mui/lab/LoadingButton";
export interface ExternalAuthProps {
displayName: string;
displayIcon: string;
authenticated: boolean;
authenticateURL: string;
externalAuthPollingState: ExternalAuthPollingState;
startPollingExternalAuth: () => void;
error?: string;
message?: string;
fullWidth?: boolean;
}
export const ExternalAuth: FC<ExternalAuthProps> = ({
displayName,
displayIcon,
authenticated,
authenticateURL,
externalAuthPollingState,
startPollingExternalAuth,
error,
message,
fullWidth = true,
}) => {
const messageContent =
message ??
(authenticated
? `Authenticated with ${displayName}`
: `Login with ${displayName}`);
return (
<>
<Tooltip
title={authenticated && `${displayName} has already been connected.`}
>
<Stack
alignItems="center"
spacing={1}
css={!fullWidth && { display: "inline-block" }}
>
<LoadingButton
loading={externalAuthPollingState === "polling"}
href={authenticateURL}
variant="contained"
size="large"
startIcon={
displayIcon && (
<img
src={displayIcon}
alt={`${displayName} Icon`}
width={16}
height={16}
/>
)
}
disabled={authenticated}
css={{ height: 42 }}
fullWidth={fullWidth}
onClick={(event) => {
event.preventDefault();
// If the user is already authenticated, we don't want to redirect them
if (authenticated || authenticateURL === "") {
return;
}
window.open(authenticateURL, "_blank", "width=900,height=600");
startPollingExternalAuth();
}}
>
{messageContent}
</LoadingButton>
{externalAuthPollingState === "abandoned" && (
<Button variant="text" onClick={startPollingExternalAuth}>
<ReplayIcon /> Check again
</Button>
)}
</Stack>
</Tooltip>
{error && (
<FormHelperText
css={(theme) => ({ color: theme.experimental.roles.error.text })}
>
{error}
</FormHelperText>
)}
</>
);
};
@@ -0,0 +1,108 @@
import { TemplateVersionExternalAuth } from "api/typesGenerated";
import { ExternalAuthButton } from "./ExternalAuthButton";
import type { Meta, StoryObj } from "@storybook/react";
const MockExternalAuth: TemplateVersionExternalAuth = {
id: "",
type: "",
display_name: "GitHub",
display_icon: "/icon/github.svg",
authenticate_url: "",
authenticated: false,
};
const meta: Meta<typeof ExternalAuthButton> = {
title: "pages/CreateWorkspacePage/ExternalAuth",
component: ExternalAuthButton,
};
export default meta;
type Story = StoryObj<typeof ExternalAuthButton>;
export const Github: Story = {
args: {
auth: MockExternalAuth,
},
};
export const GithubWithRetry: Story = {
args: {
auth: MockExternalAuth,
displayRetry: true,
},
};
export const GithubAuthenticated: Story = {
args: {
auth: {
...MockExternalAuth,
authenticated: true,
},
},
};
export const Gitlab: Story = {
args: {
auth: {
...MockExternalAuth,
display_icon: "/icon/gitlab.svg",
display_name: "GitLab",
authenticated: false,
},
},
};
export const GitlabAuthenticated: Story = {
args: {
auth: {
...MockExternalAuth,
display_icon: "/icon/gitlab.svg",
display_name: "GitLab",
authenticated: true,
},
},
};
export const AzureDevOps: Story = {
args: {
auth: {
...MockExternalAuth,
display_icon: "/icon/azure-devops.svg",
display_name: "Azure DevOps",
authenticated: false,
},
},
};
export const AzureDevOpsAuthenticated: Story = {
args: {
auth: {
...MockExternalAuth,
display_icon: "/icon/azure-devops.svg",
display_name: "Azure DevOps",
authenticated: true,
},
},
};
export const Bitbucket: Story = {
args: {
auth: {
...MockExternalAuth,
display_icon: "/icon/bitbucket.svg",
display_name: "Bitbucket",
authenticated: false,
},
},
};
export const BitbucketAuthenticated: Story = {
args: {
auth: {
...MockExternalAuth,
display_icon: "/icon/bitbucket.svg",
display_name: "Bitbucket",
authenticated: true,
},
},
};
@@ -0,0 +1,74 @@
import ReplayIcon from "@mui/icons-material/Replay";
import Button from "@mui/material/Button";
import Tooltip from "@mui/material/Tooltip";
import { type FC } from "react";
import LoadingButton from "@mui/lab/LoadingButton";
import { visuallyHidden } from "@mui/utils";
import { ExternalImage } from "components/ExternalImage/ExternalImage";
import { TemplateVersionExternalAuth } from "api/typesGenerated";
export interface ExternalAuthButtonProps {
auth: TemplateVersionExternalAuth;
displayRetry: boolean;
isLoading: boolean;
onStartPolling: () => void;
}
export const ExternalAuthButton: FC<ExternalAuthButtonProps> = ({
auth,
displayRetry,
isLoading,
onStartPolling,
}) => {
return (
<>
<div css={{ display: "flex", alignItems: "center", gap: 8 }}>
<LoadingButton
fullWidth
loading={isLoading}
href={auth.authenticate_url}
variant="contained"
size="xlarge"
startIcon={
auth.display_icon && (
<ExternalImage
src={auth.display_icon}
alt={`${auth.display_name} Icon`}
css={{ width: 16, height: 16 }}
/>
)
}
disabled={auth.authenticated}
onClick={() => {
window.open(
auth.authenticate_url,
"_blank",
"width=900,height=600",
);
onStartPolling();
}}
>
{auth.authenticated
? `Authenticated with ${auth.display_name}`
: `Login with ${auth.display_name}`}
</LoadingButton>
{displayRetry && (
<Tooltip title="Retry">
<Button
variant="contained"
size="xlarge"
onClick={onStartPolling}
css={{ minWidth: "auto", aspectRatio: "1" }}
>
<ReplayIcon css={{ width: 20, height: 20 }} />
<span aria-hidden css={{ ...visuallyHidden }}>
Refresh external auth
</span>
</Button>
</Tooltip>
)}
</div>
</>
);
};