Skip to content

Commit

Permalink
Remove experimentation flags for OAuth (#22554)
Browse files Browse the repository at this point in the history
  • Loading branch information
timroes authored Feb 8, 2023
1 parent 3694eb9 commit 9834ca1
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 72 deletions.
4 changes: 0 additions & 4 deletions airbyte-webapp/src/hooks/services/Experiment/experiments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ export interface Experiments {
"authPage.hideSelfHostedCTA": boolean;
"authPage.signup.hideName": boolean;
"authPage.signup.hideCompanyName": boolean;
"authPage.oauth.google": boolean;
"authPage.oauth.github": boolean;
"authPage.oauth.google.signUpPage": boolean;
"authPage.oauth.github.signUpPage": boolean;
"onboarding.speedyConnection": boolean;
"authPage.oauth.position": "top" | "bottom";
"connection.onboarding.sources": string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { EMPTY } from "rxjs";
import { TestWrapper } from "test-utils/testutils";

import type { useExperiment } from "hooks/services/Experiment";
import type { Experiments } from "hooks/services/Experiment/experiments";

const mockUseExperiment = jest.fn<ReturnType<typeof useExperiment>, Parameters<typeof useExperiment>>();
jest.doMock("hooks/services/Experiment", () => ({
Expand All @@ -22,28 +21,6 @@ jest.doMock("packages/cloud/services/auth/AuthService", () => ({
// eslint-disable-next-line @typescript-eslint/no-var-requires
const { OAuthLogin } = require("./OAuthLogin");

const createUseExperimentMock = (options: {
google?: boolean;
github?: boolean;
googleSignUp?: boolean;
githubSignUp?: boolean;
}) => {
return (key: keyof Experiments) => {
switch (key) {
case "authPage.oauth.github":
return options.github ?? false;
case "authPage.oauth.google":
return options.google ?? false;
case "authPage.oauth.github.signUpPage":
return options.githubSignUp ?? true;
case "authPage.oauth.google.signUpPage":
return options.googleSignUp ?? true;
default:
throw new Error(`${key} is not mocked`);
}
};
};

describe("OAuthLogin", () => {
beforeEach(() => {
mockUseExperiment.mockReset();
Expand All @@ -52,27 +29,6 @@ describe("OAuthLogin", () => {
mockLoginWithOAuth.mockReturnValue(EMPTY);
});

it("should render all enabled logins", () => {
mockUseExperiment.mockImplementation(createUseExperimentMock({ google: true, github: true }));
const { getByTestId } = render(<OAuthLogin />, { wrapper: TestWrapper });
expect(getByTestId("googleOauthLogin")).toBeInTheDocument();
expect(getByTestId("githubOauthLogin")).toBeInTheDocument();
});

it("should not render buttons that are disabled", () => {
mockUseExperiment.mockImplementation(createUseExperimentMock({ google: false, github: true }));
const { getByTestId, queryByTestId } = render(<OAuthLogin />, { wrapper: TestWrapper });
expect(queryByTestId("googleOauthLogin")).not.toBeInTheDocument();
expect(getByTestId("githubOauthLogin")).toBeInTheDocument();
});

it("should not render disabled buttons for sign-up page", () => {
mockUseExperiment.mockImplementation(createUseExperimentMock({ google: true, github: true, googleSignUp: false }));
const { getByTestId, queryByTestId } = render(<OAuthLogin isSignUpPage />, { wrapper: TestWrapper });
expect(queryByTestId("googleOauthLogin")).not.toBeInTheDocument();
expect(getByTestId("githubOauthLogin")).toBeInTheDocument();
});

it("should call auth service for Google", () => {
const { getByTestId } = render(<OAuthLogin />, { wrapper: TestWrapper });
userEvents.click(getByTestId("googleOauthLogin"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { Subscription } from "rxjs";

import { Spinner } from "components/ui/Spinner";

import { useExperiment } from "hooks/services/Experiment";
import { OAuthProviders } from "packages/cloud/lib/auth/AuthProviders";
import { useAuthService } from "packages/cloud/services/auth/AuthService";

Expand All @@ -31,35 +30,17 @@ const GoogleButton: React.FC<{ onClick: () => void }> = ({ onClick }) => {
);
};

interface OAuthLoginProps {
isSignUpPage?: boolean;
}

export const OAuthLogin: React.FC<OAuthLoginProps> = ({ isSignUpPage }) => {
export const OAuthLogin: React.FC = () => {
const { formatMessage } = useIntl();
const { loginWithOAuth } = useAuthService();
const stateSubscription = useRef<Subscription>();
const [errorCode, setErrorCode] = useState<string>();
const [isLoading, setLoading] = useState(false);

const isGitHubLoginEnabled = useExperiment("authPage.oauth.github", true);
const isGoogleLoginEnabled = useExperiment("authPage.oauth.google", true);
const isGitHubEnabledOnSignUp = useExperiment("authPage.oauth.github.signUpPage", true);
const isGoogleEnabledOnSignUp = useExperiment("authPage.oauth.google.signUpPage", true);

const showGoogleLogin = isGoogleLoginEnabled && (!isSignUpPage || isGoogleEnabledOnSignUp);
const showGitHubLogin = isGitHubLoginEnabled && (!isSignUpPage || isGitHubEnabledOnSignUp);

const isAnyOauthEnabled = showGoogleLogin || showGitHubLogin;

useUnmount(() => {
stateSubscription.current?.unsubscribe();
});

if (!isAnyOauthEnabled) {
return null;
}

const getErrorMessage = (error: string): string | undefined => {
switch (error) {
// The following error codes are not really errors, thus we'll ignore them.
Expand Down Expand Up @@ -107,8 +88,8 @@ export const OAuthLogin: React.FC<OAuthLoginProps> = ({ isSignUpPage }) => {
)}
{!isLoading && (
<div className={styles.buttons}>
{showGoogleLogin && <GoogleButton onClick={() => login("google")} />}
{showGitHubLogin && <GitHubButton onClick={() => login("github")} />}
<GoogleButton onClick={() => login("google")} />
<GitHubButton onClick={() => login("github")} />
</div>
)}
{errorMessage && <div className={styles.error}>{errorMessage}</div>}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ const SignupPage: React.FC<SignupPageProps> = ({ highlightStyle }) => {
<SpecialBlock />
{oAuthPosition === "top" && (
<>
<OAuthLogin isSignUpPage />
<OAuthLogin />
<Separator />
</>
)}
<SignupForm />
{oAuthPosition === "bottom" && (
<>
<Separator />
<OAuthLogin isSignUpPage />
<OAuthLogin />
</>
)}
<Disclaimer />
Expand Down

0 comments on commit 9834ca1

Please sign in to comment.