From 4ee3badcb225e952c5fa1c6480bf5fbdb361ca9e Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Wed, 21 Feb 2024 23:46:11 +0000 Subject: [PATCH 1/4] changes from base --- .../CreateWorkspacePage.test.tsx | 107 +++++--- .../CreateWorkspacePageView.stories.tsx | 2 +- .../CreateWorkspacePageView.tsx | 256 ++++++++++-------- .../ExternalAuthBanner.stories.tsx | 34 --- .../ExternalAuthBanner/ExternalAuthBanner.tsx | 91 ------- .../ExternalAuthItem.stories.tsx | 50 ---- .../ExternalAuthItem.test.tsx | 62 ----- .../ExternalAuthBanner/ExternalAuthItem.tsx | 124 --------- .../ExternalAuthButton.stories.tsx | 108 ++++++++ .../ExternalAuthButton.tsx | 80 ++++++ 10 files changed, 392 insertions(+), 522 deletions(-) delete mode 100644 site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthBanner.stories.tsx delete mode 100644 site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthBanner.tsx delete mode 100644 site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthItem.stories.tsx delete mode 100644 site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthItem.test.tsx delete mode 100644 site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthItem.tsx create mode 100644 site/src/pages/CreateWorkspacePage/ExternalAuthButton.stories.tsx create mode 100644 site/src/pages/CreateWorkspacePage/ExternalAuthButton.tsx diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index 61482cbb0f7a2..76f86b4bff53d 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -6,12 +6,14 @@ import { MockUser, MockWorkspace, MockWorkspaceQuota, + MockWorkspaceRequest, MockWorkspaceRichParametersRequest, MockTemplateVersionParameter1, MockTemplateVersionParameter2, MockTemplateVersionParameter3, MockTemplateVersionExternalAuthGithub, MockOrganization, + MockTemplateVersionExternalAuthGithubAuthenticated, } from "testHelpers/entities"; import { renderWithAuth, @@ -19,8 +21,6 @@ import { } from "testHelpers/renderHelpers"; import CreateWorkspacePage from "./CreateWorkspacePage"; import { Language } from "./CreateWorkspacePageView"; -import { server } from "testHelpers/server"; -import { rest } from "msw"; const nameLabelText = "Workspace Name"; const createWorkspaceText = "Create Workspace"; @@ -185,6 +185,67 @@ describe("CreateWorkspacePage", () => { expect(validationError).toBeInTheDocument(); }); + it("external auth authenticates and succeeds", async () => { + jest + .spyOn(API, "getWorkspaceQuota") + .mockResolvedValueOnce(MockWorkspaceQuota); + jest + .spyOn(API, "getUsers") + .mockResolvedValueOnce({ users: [MockUser], count: 1 }); + jest.spyOn(API, "createWorkspace").mockResolvedValueOnce(MockWorkspace); + jest + .spyOn(API, "getTemplateVersionExternalAuth") + .mockResolvedValue([MockTemplateVersionExternalAuthGithub]); + + renderCreateWorkspacePage(); + await waitForLoaderToBeRemoved(); + + const nameField = await screen.findByLabelText(nameLabelText); + // have to use fireEvent b/c userEvent isn't cleaning up properly between tests + fireEvent.change(nameField, { + target: { value: "test" }, + }); + + const githubButton = await screen.findByText("Login with GitHub"); + await userEvent.click(githubButton); + + jest + .spyOn(API, "getTemplateVersionExternalAuth") + .mockResolvedValue([MockTemplateVersionExternalAuthGithubAuthenticated]); + + await screen.findByText( + "Authenticated with GitHub", + {}, + { interval: 500, timeout: 5000 }, + ); + + const submitButton = screen.getByText(createWorkspaceText); + await userEvent.click(submitButton); + + await waitFor(() => + expect(API.createWorkspace).toBeCalledWith( + MockUser.organization_ids[0], + MockUser.id, + expect.objectContaining({ + ...MockWorkspaceRequest, + }), + ), + ); + }); + + it("external auth errors if unauthenticated", async () => { + jest + .spyOn(API, "getTemplateVersionExternalAuth") + .mockResolvedValueOnce([MockTemplateVersionExternalAuthGithub]); + + renderCreateWorkspacePage(); + await waitForLoaderToBeRemoved(); + + await screen.findByText( + "To create a workspace using the selected template, please ensure you are authenticated with all the external providers listed below.", + ); + }); + it("auto create a workspace if uses mode=auto", async () => { const param = "first_parameter"; const paramValue = "It works!"; @@ -259,46 +320,4 @@ describe("CreateWorkspacePage", () => { expect(warningMessage).toHaveTextContent(Language.duplicationWarning); expect(nameInput).toHaveValue(`${MockWorkspace.name}-copy`); }); - - it("displays the form after connecting to all the external services", async () => { - jest.spyOn(window, "open").mockImplementation(() => null); - const user = userEvent.setup(); - const notAuthenticatedExternalAuth = { - ...MockTemplateVersionExternalAuthGithub, - authenticated: false, - }; - server.use( - rest.get( - "/api/v2/templateversions/:versionId/external-auth", - (req, res, ctx) => { - return res(ctx.json([notAuthenticatedExternalAuth])); - }, - ), - ); - renderCreateWorkspacePage(); - - await screen.findByText("External authentication"); - expect(screen.queryByRole("form")).not.toBeInTheDocument(); - - const connectButton = screen.getByRole("button", { - name: /connect/i, - }); - server.use( - rest.get( - "/api/v2/templateversions/:versionId/external-auth", - (req, res, ctx) => { - const authenticatedExternalAuth = { - ...MockTemplateVersionExternalAuthGithub, - authenticated: true, - }; - return res(ctx.json([authenticatedExternalAuth])); - }, - ), - ); - await user.click(connectButton); - // TODO: Consider improving the timeout by simulating react-query polling. - // Current implementation could not achieve this, further research is - // needed. - await screen.findByRole("form", undefined, { timeout: 10_000 }); - }); }); diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx index 5662f96338ef6..5e24527f46541 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx @@ -108,7 +108,7 @@ export const Parameters: Story = { }, }; -export const RequiresExternalAuth: Story = { +export const ExternalAuth: Story = { args: { externalAuth: [ { diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index 2bc393121c282..5cbb3f06d40fe 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -25,6 +25,7 @@ import { getInitialRichParameterValues, useValidationSchemaForRichParameters, } from "utils/richParameters"; +import { ExternalAuthButton } from "./ExternalAuthButton"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { Stack } from "components/Stack/Stack"; import { Alert } from "components/Alert/Alert"; @@ -42,7 +43,6 @@ import { CreateWorkspaceMode, ExternalAuthPollingState, } from "./CreateWorkspacePage"; -import { ExternalAuthBanner } from "./ExternalAuthBanner/ExternalAuthBanner"; import { CreateWSPermissions } from "./permissions"; export const Language = { @@ -147,6 +147,10 @@ export const CreateWorkspacePageView: FC = ({ ); }, [autofillParameters]); + const hasAllRequiredExternalAuth = externalAuth.every( + (auth) => auth.optional || auth.authenticated, + ); + return ( Cancel}> @@ -171,134 +175,154 @@ export const CreateWorkspacePageView: FC = ({ - {requiresExternalAuth ? ( - - ) : ( - - {Boolean(error) && } + + {Boolean(error) && } - {mode === "duplicate" && ( - - {Language.duplicationWarning} - - )} + {mode === "duplicate" && ( + + {Language.duplicationWarning} + + )} - {/* General info */} - - - {versionId && versionId !== template.active_version_id && ( - - - - This parameter has been preset, and cannot be modified. - - - )} -
+ {/* General info */} + + + {versionId && versionId !== template.active_version_id && ( + - - Need a suggestion?{" "} - - -
+ + This parameter has been preset, and cannot be modified. + + + )} - {permissions.createWorkspaceForUser && ( - { - setOwner(user ?? defaultOwner); +
+ + + Need a suggestion?{" "} + + +
+ + {permissions.createWorkspaceForUser && ( + { + setOwner(user ?? defaultOwner); + }} + label="Owner" + size="medium" + /> + )} +
+
+ + {externalAuth && externalAuth.length > 0 && ( + + + {hasAllRequiredExternalAuth ? ( + + This template can connect to the external authentication + providers listed below. + + ) : ( + + To create a workspace using this template, please connect to + all required external authentication providers listed below. + )} + {externalAuth.map((auth) => ( + + ))} + )} - {parameters.length > 0 && ( - - {/* The parameter fields are densely packed and carry significant information, - /* hence they require additional vertical spacing for better - /*readability and user experience. */} - - {parameters.map((parameter, index) => { - const parameterField = `rich_parameter_values.${index}`; - const parameterInputName = `${parameterField}.value`; - const isDisabled = - disabledParamsList?.includes( - parameter.name.toLowerCase().replace(/ /g, "_"), - ) || creatingWorkspace; + {parameters.length > 0 && ( + + {/* The parameter fields are densely packed and carry significant information, + hence they require additional vertical spacing for better readability and + user experience. */} + + {parameters.map((parameter, index) => { + const parameterField = `rich_parameter_values.${index}`; + const parameterInputName = `${parameterField}.value`; + const isDisabled = + disabledParamsList?.includes( + parameter.name.toLowerCase().replace(/ /g, "_"), + ) || creatingWorkspace; - return ( - { - await form.setFieldValue(parameterField, { - name: parameter.name, - value, - }); - }} - autofillSource={autofillSources[parameter.name]} - key={parameter.name} - parameter={parameter} - disabled={isDisabled} - /> - ); - })} - - - )} + return ( + { + await form.setFieldValue(parameterField, { + name: parameter.name, + value, + }); + }} + autofillSource={autofillSources[parameter.name]} + key={parameter.name} + parameter={parameter} + disabled={isDisabled} + /> + ); + })} + + + )} - -
- )} + +
); }; diff --git a/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthBanner.stories.tsx b/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthBanner.stories.tsx deleted file mode 100644 index 2d9c4e9c45359..0000000000000 --- a/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthBanner.stories.tsx +++ /dev/null @@ -1,34 +0,0 @@ -import { TemplateVersionExternalAuth } from "api/typesGenerated"; -import { ExternalAuthBanner } from "./ExternalAuthBanner"; -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 = { - title: "pages/CreateWorkspacePage/ExternalAuthBanner", - component: ExternalAuthBanner, -}; - -export default meta; -type Story = StoryObj; - -export const Default: Story = { - args: { - providers: [ - MockExternalAuth, - { - ...MockExternalAuth, - display_name: "Google", - display_icon: "/icon/google.svg", - authenticated: true, - }, - ], - }, -}; diff --git a/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthBanner.tsx b/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthBanner.tsx deleted file mode 100644 index ee8e4b47f546d..0000000000000 --- a/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthBanner.tsx +++ /dev/null @@ -1,91 +0,0 @@ -import { Interpolation, Theme } from "@emotion/react"; -import { TemplateVersionExternalAuth } from "api/typesGenerated"; -import { ExternalAuthPollingState } from "../CreateWorkspacePage"; -import { ExternalAuthItem } from "./ExternalAuthItem"; -import { FC } from "react"; - -type ExternalAuthBannerProps = { - providers: TemplateVersionExternalAuth[]; - pollingState: ExternalAuthPollingState; - onStartPolling: () => void; -}; - -export const ExternalAuthBanner: FC = ({ - providers, - pollingState, - onStartPolling, -}) => { - return ( -
-
-
-

External authentication

-

- To create a workspace using the selected template, please ensure you - are connected with all the external services. -

-
- -
    - {providers.map((p) => ( - - ))} -
-
-
- ); -}; - -const styles = { - root: (theme) => ({ - display: "flex", - alignItems: "center", - justifyContent: "center", - padding: 48, - minHeight: 460, - border: `1px solid ${theme.palette.divider}`, - borderRadius: 8, - lineHeight: "1.5", - }), - - header: { - textAlign: "center", - // Better text distribution - maxWidth: 324, - margin: "auto", - }, - - content: { - maxWidth: 380, - }, - - title: { - fontSize: 20, - fontWeight: 400, - margin: 0, - lineHeight: "1.2", - }, - - description: (theme) => ({ - margin: 0, - marginTop: 12, - fontSize: 14, - color: theme.palette.text.secondary, - }), - - providerList: { - listStyle: "none", - padding: 0, - margin: 0, - display: "flex", - flexDirection: "column", - gap: 8, - marginTop: 24, - }, -} as Record>; diff --git a/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthItem.stories.tsx b/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthItem.stories.tsx deleted file mode 100644 index a60b3e317c19e..0000000000000 --- a/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthItem.stories.tsx +++ /dev/null @@ -1,50 +0,0 @@ -import { TemplateVersionExternalAuth } from "api/typesGenerated"; -import { ExternalAuthItem } from "./ExternalAuthItem"; -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 = { - title: "pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthItem", - component: ExternalAuthItem, - decorators: [ - (Story) => ( -
- -
- ), - ], -}; - -export default meta; -type Story = StoryObj; - -export const Default: Story = { - args: { - provider: MockExternalAuth, - }, -}; - -export const Connected: Story = { - args: { - provider: { - ...MockExternalAuth, - authenticated: true, - }, - }, -}; - -export const Connecting: Story = { - args: { - provider: MockExternalAuth, - defaultStatus: "connecting", - isPolling: true, - }, -}; diff --git a/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthItem.test.tsx b/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthItem.test.tsx deleted file mode 100644 index f0d0871b9f7e3..0000000000000 --- a/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthItem.test.tsx +++ /dev/null @@ -1,62 +0,0 @@ -import { render, screen } from "@testing-library/react"; -import { ExternalAuthItem } from "./ExternalAuthItem"; -import { ThemeProvider } from "contexts/ThemeProvider"; -import { TemplateVersionExternalAuth } from "api/typesGenerated"; -import userEvent from "@testing-library/user-event"; - -jest.spyOn(window, "open").mockImplementation(() => null); - -const MockExternalAuth: TemplateVersionExternalAuth = { - id: "", - type: "", - display_name: "GitHub", - display_icon: "/icon/github.svg", - authenticate_url: "", - authenticated: false, -}; - -test("changes to idle when polling stops", async () => { - const user = userEvent.setup(); - const startPollingFn = jest.fn(); - const { rerender } = render( - , - { wrapper: ThemeProvider }, - ); - - const connectButton = screen.getByText(/connect/i); - expect(isLoading(connectButton)).toBeFalsy(); - - await user.click(connectButton); - expect(startPollingFn).toHaveBeenCalledTimes(1); - expect(window.open).toHaveBeenCalledTimes(1); - - rerender( - , - ); - - // Check if the button is loading - screen.getByRole("progressbar"); - - rerender( - , - ); - - expect(isLoading(connectButton)).toBeFalsy(); -}); - -function isLoading(el: HTMLButtonElement) { - const progressBar = el.querySelector('[role="progressbar"]'); - return Boolean(progressBar); -} diff --git a/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthItem.tsx b/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthItem.tsx deleted file mode 100644 index 45d5e6057875e..0000000000000 --- a/site/src/pages/CreateWorkspacePage/ExternalAuthBanner/ExternalAuthItem.tsx +++ /dev/null @@ -1,124 +0,0 @@ -import { Interpolation, Theme } from "@emotion/react"; -import DoneAllOutlined from "@mui/icons-material/DoneAllOutlined"; -import LoadingButton from "@mui/lab/LoadingButton"; -import { TemplateVersionExternalAuth } from "api/typesGenerated"; -import { ExternalImage } from "components/ExternalImage/ExternalImage"; -import { FC, useEffect, useState } from "react"; -// eslint-disable-next-line no-restricted-imports -- used to allow extension with "component" -import Box, { BoxProps } from "@mui/material/Box"; - -type Status = "idle" | "connecting"; - -type ExternalAuthItemProps = { - provider: TemplateVersionExternalAuth; - isPolling: boolean; - defaultStatus?: Status; - onStartPolling: () => void; -} & BoxProps; - -export const ExternalAuthItem: FC = ({ - provider, - isPolling, - defaultStatus = "idle", - onStartPolling, - ...boxProps -}) => { - const [status, setStatus] = useState(defaultStatus); - - useEffect(() => { - if (!isPolling) { - setStatus("idle"); - } - }, [isPolling]); - - return ( - - - - {provider.display_name} - - {provider.authenticated ? ( - - Connected - - - ) : ( - { - setStatus("connecting"); - window.open( - provider.authenticate_url, - "_blank", - "width=900,height=600", - ); - onStartPolling(); - }} - > - Connect… - - )} - - ); -}; - -const styles = { - providerItem: (theme) => ({ - display: "flex", - alignItems: "center", - padding: "8px 8px 8px 20px", - border: `1px solid ${theme.palette.divider}`, - borderRadius: 6, - justifyContent: "space-between", - gap: 24, - fontSize: 14, - }), - - providerHeader: { - display: "flex", - alignItems: "center", - gap: 12, - flex: 1, - overflow: "hidden", - }, - - providerName: { - fontWeight: 500, - display: "block", - whiteSpace: "nowrap", - maxWidth: "100%", - textOverflow: "ellipsis", - overflow: "hidden", - }, - - providerIcon: { - width: 16, - height: 16, - }, - - connectButton: { - flexShrink: 0, - borderRadius: 4, - }, - - providerConnectedLabel: (theme) => ({ - fontSize: 13, - display: "flex", - alignItems: "center", - color: theme.palette.text.disabled, - gap: 8, - // Have the same height of the button - height: 32, - // Better visual alignment - padding: "0 8px", - }), - - providerConnectedLabelIcon: (theme) => ({ - color: theme.roles.success.fill.solid, - fontSize: 16, - }), -} as Record>; diff --git a/site/src/pages/CreateWorkspacePage/ExternalAuthButton.stories.tsx b/site/src/pages/CreateWorkspacePage/ExternalAuthButton.stories.tsx new file mode 100644 index 0000000000000..97c9d743552ad --- /dev/null +++ b/site/src/pages/CreateWorkspacePage/ExternalAuthButton.stories.tsx @@ -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 = { + title: "pages/CreateWorkspacePage/ExternalAuth", + component: ExternalAuthButton, +}; + +export default meta; +type Story = StoryObj; + +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, + }, + }, +}; diff --git a/site/src/pages/CreateWorkspacePage/ExternalAuthButton.tsx b/site/src/pages/CreateWorkspacePage/ExternalAuthButton.tsx new file mode 100644 index 0000000000000..3357450688fa9 --- /dev/null +++ b/site/src/pages/CreateWorkspacePage/ExternalAuthButton.tsx @@ -0,0 +1,80 @@ +import ReplayIcon from "@mui/icons-material/Replay"; +import Button from "@mui/material/Button"; +import Tooltip from "@mui/material/Tooltip"; +import LoadingButton from "@mui/lab/LoadingButton"; +import { visuallyHidden } from "@mui/utils"; +import { type FC } from "react"; +import type { TemplateVersionExternalAuth } from "api/typesGenerated"; +import { ExternalImage } from "components/ExternalImage/ExternalImage"; +import { Pill } from "components/Pill/Pill"; + +export interface ExternalAuthButtonProps { + auth: TemplateVersionExternalAuth; + displayRetry: boolean; + isLoading: boolean; + onStartPolling: () => void; +} + +export const ExternalAuthButton: FC = ({ + auth, + displayRetry, + isLoading, + onStartPolling, +}) => { + return ( + <> +
+ + ) + } + 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}`} + {!auth.optional && !auth.authenticated && ( + + Required + + )} + + + {displayRetry && ( + + + + )} +
+ + ); +}; From c04cd378cf352aa201f8fe0c94e4fb038db2a4b7 Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Wed, 21 Feb 2024 23:47:01 +0000 Subject: [PATCH 2/4] support optional external auth providers in the frontend --- .../CreateWorkspacePage.test.tsx | 6 +- .../CreateWorkspacePageView.stories.tsx | 74 +++++++++++++++++++ .../CreateWorkspacePageView.tsx | 7 +- .../ExternalAuthButton.stories.tsx | 14 +++- .../ExternalAuthButton.tsx | 20 +++-- 5 files changed, 106 insertions(+), 15 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index 76f86b4bff53d..11899a78966fc 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -241,9 +241,7 @@ describe("CreateWorkspacePage", () => { renderCreateWorkspacePage(); await waitForLoaderToBeRemoved(); - await screen.findByText( - "To create a workspace using the selected template, please ensure you are authenticated with all the external providers listed below.", - ); + await screen.findByText(/connect to all required/i); }); it("auto create a workspace if uses mode=auto", async () => { @@ -312,7 +310,7 @@ describe("CreateWorkspacePage", () => { route: `/templates/${MockWorkspace.name}/workspace?${params.toString()}`, }); - const warningMessage = await screen.findByRole("alert"); + const warningMessage = await screen.findByTestId("duplication-warning"); const nameInput = await screen.findByRole("textbox", { name: "Workspace Name", }); diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx index 5e24527f46541..9c9d64be0c8db 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx @@ -126,6 +126,80 @@ export const ExternalAuth: Story = { authenticate_url: "", display_icon: "/icon/gitlab.svg", display_name: "GitLab", + optional: true, + }, + ], + }, +}; + +export const ExternalAuthError: Story = { + args: { + error: true, + externalAuth: [ + { + id: "github", + type: "github", + authenticated: false, + authenticate_url: "", + display_icon: "/icon/github.svg", + display_name: "GitHub", + }, + { + id: "gitlab", + type: "gitlab", + authenticated: false, + authenticate_url: "", + display_icon: "/icon/gitlab.svg", + display_name: "GitLab", + optional: true, + }, + ], + }, +}; + +export const ExternalAuthAllRequiredConnected: Story = { + args: { + externalAuth: [ + { + id: "github", + type: "github", + authenticated: true, + authenticate_url: "", + display_icon: "/icon/github.svg", + display_name: "GitHub", + }, + { + id: "gitlab", + type: "gitlab", + authenticated: false, + authenticate_url: "", + display_icon: "/icon/gitlab.svg", + display_name: "GitLab", + optional: true, + }, + ], + }, +}; + +export const ExternalAuthAllConnected: Story = { + args: { + externalAuth: [ + { + id: "github", + type: "github", + authenticated: true, + authenticate_url: "", + display_icon: "/icon/github.svg", + display_name: "GitHub", + }, + { + id: "gitlab", + type: "gitlab", + authenticated: true, + authenticate_url: "", + display_icon: "/icon/gitlab.svg", + display_name: "GitLab", + optional: true, }, ], }, diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index 5cbb3f06d40fe..7739838c93e9c 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -183,7 +183,7 @@ export const CreateWorkspacePageView: FC = ({ {Boolean(error) && } {mode === "duplicate" && ( - + {Language.duplicationWarning} )} @@ -252,7 +252,7 @@ export const CreateWorkspacePageView: FC = ({ {externalAuth && externalAuth.length > 0 && ( {hasAllRequiredExternalAuth ? ( @@ -261,7 +261,7 @@ export const CreateWorkspacePageView: FC = ({ providers listed below. ) : ( - + To create a workspace using this template, please connect to all required external authentication providers listed below. @@ -269,6 +269,7 @@ export const CreateWorkspacePageView: FC = ({ {externalAuth.map((auth) => ( = { - title: "pages/CreateWorkspacePage/ExternalAuth", + title: "pages/CreateWorkspacePage/ExternalAuthButton", component: ExternalAuthButton, }; @@ -25,6 +25,15 @@ export const Github: Story = { }, }; +export const GithubOptional: Story = { + args: { + auth: { + ...MockExternalAuth, + optional: true, + }, + }, +}; + export const GithubWithRetry: Story = { args: { auth: MockExternalAuth, @@ -48,6 +57,7 @@ export const Gitlab: Story = { display_icon: "/icon/gitlab.svg", display_name: "GitLab", authenticated: false, + optional: true, }, }, }; @@ -70,6 +80,7 @@ export const AzureDevOps: Story = { display_icon: "/icon/azure-devops.svg", display_name: "Azure DevOps", authenticated: false, + optional: true, }, }, }; @@ -92,6 +103,7 @@ export const Bitbucket: Story = { display_icon: "/icon/bitbucket.svg", display_name: "Bitbucket", authenticated: false, + optional: true, }, }, }; diff --git a/site/src/pages/CreateWorkspacePage/ExternalAuthButton.tsx b/site/src/pages/CreateWorkspacePage/ExternalAuthButton.tsx index 3357450688fa9..18e0f45bbd0ab 100644 --- a/site/src/pages/CreateWorkspacePage/ExternalAuthButton.tsx +++ b/site/src/pages/CreateWorkspacePage/ExternalAuthButton.tsx @@ -13,6 +13,7 @@ export interface ExternalAuthButtonProps { displayRetry: boolean; isLoading: boolean; onStartPolling: () => void; + error?: unknown; } export const ExternalAuthButton: FC = ({ @@ -20,6 +21,7 @@ export const ExternalAuthButton: FC = ({ displayRetry, isLoading, onStartPolling, + error, }) => { return ( <> @@ -49,13 +51,17 @@ export const ExternalAuthButton: FC = ({ onStartPolling(); }} > - {auth.authenticated - ? `Authenticated with ${auth.display_name}` - : `Login with ${auth.display_name}`} - {!auth.optional && !auth.authenticated && ( - - Required - + {auth.authenticated ? ( + `Authenticated with ${auth.display_name}` + ) : ( + <> + Login with {auth.display_name} + {!auth.optional && ( + + Required + + )} + )} From f00b9910e6641270f83be6cfe021fe17ef506970 Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Wed, 21 Feb 2024 23:55:17 +0000 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=A7=B9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../CreateWorkspacePage/CreateWorkspacePageView.tsx | 9 ++------- .../src/pages/CreateWorkspacePage/ExternalAuthButton.tsx | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index 7739838c93e9c..a3ef3163775c8 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -255,13 +255,8 @@ export const CreateWorkspacePageView: FC = ({ description="This template uses external services for authentication." > - {hasAllRequiredExternalAuth ? ( - - This template can connect to the external authentication - providers listed below. - - ) : ( - + {Boolean(error) && !hasAllRequiredExternalAuth && ( + To create a workspace using this template, please connect to all required external authentication providers listed below. diff --git a/site/src/pages/CreateWorkspacePage/ExternalAuthButton.tsx b/site/src/pages/CreateWorkspacePage/ExternalAuthButton.tsx index 18e0f45bbd0ab..331ea0f22f9e4 100644 --- a/site/src/pages/CreateWorkspacePage/ExternalAuthButton.tsx +++ b/site/src/pages/CreateWorkspacePage/ExternalAuthButton.tsx @@ -57,7 +57,7 @@ export const ExternalAuthButton: FC = ({ <> Login with {auth.display_name} {!auth.optional && ( - + Required )} From 5ba780d1d8f79e543de7f5aeae86afcc95d19c11 Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Wed, 21 Feb 2024 23:59:27 +0000 Subject: [PATCH 4/4] remove unnecessary test --- .../CreateWorkspacePage/CreateWorkspacePage.test.tsx | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index 11899a78966fc..bc48e8e74d81f 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -233,17 +233,6 @@ describe("CreateWorkspacePage", () => { ); }); - it("external auth errors if unauthenticated", async () => { - jest - .spyOn(API, "getTemplateVersionExternalAuth") - .mockResolvedValueOnce([MockTemplateVersionExternalAuthGithub]); - - renderCreateWorkspacePage(); - await waitForLoaderToBeRemoved(); - - await screen.findByText(/connect to all required/i); - }); - it("auto create a workspace if uses mode=auto", async () => { const param = "first_parameter"; const paramValue = "It works!";