Skip to content

Commit

Permalink
fix tests and reorganize code to separete loaduser from login
Browse files Browse the repository at this point in the history
  • Loading branch information
Mazuh committed Oct 24, 2023
1 parent 854cb4d commit 5b8c762
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 60 deletions.
3 changes: 1 addition & 2 deletions src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { ThemeProvider } from "@mui/material/styles";
import { Switch, Route, Router, Redirect } from "wouter";
import { darkTheme } from "./components/app/mui-styles";
import { useAppDispatch } from "./state";
import { loadUser, login } from "./state/user";
import { loadUser } from "./state/user";
import AuthMain from "./features/auth/AuthMain";
import P2PCallMain from "./features/p2p-call/P2PCallMain";
import CallCreationMain from "./features/call-selection/CallCreationMain";
Expand All @@ -15,7 +15,6 @@ export default function App() {
const dispatch = useAppDispatch();

useEffect(() => {
dispatch(login());
dispatch(loadUser());
}, [dispatch]);

Expand Down
11 changes: 8 additions & 3 deletions src/features/auth/AuthMain.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,23 @@ describe("CurrentUserStateIndicator", () => {
(firestoreAuth.login as jest.Mock).mockClear();
});

it("can login", async () => {
it("login will set loading ui to prevent double trigger", async () => {
(firestoreAuth.login as jest.Mock).mockResolvedValue(
new Promise((resolve) => setTimeout(resolve, 500))
);

const { store } = fullRender(<AuthMain />);

const loginButtonElement = screen.getByRole("button", {
name: "Continue with Google",
});
await act(() => fireEvent.click(loginButtonElement));

await waitFor(() => expect(store.getState().user.uid).not.toBe(""));
await waitFor(() => expect(store.getState().user.uid).toBe(""));
await waitFor(() => expect(screen.getByText("Checking...")).toBeTruthy());
});

it("login is integrated with firestore", async () => {
it("triggering login will rely on firestore module", async () => {
fullRender(<AuthMain />);

const loginButtonElement = screen.getByRole("button", {
Expand Down
11 changes: 7 additions & 4 deletions src/features/auth/AuthMain.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ import Button from "@mui/material/Button";
import Typography from "@mui/material/Typography";
import GoogleIcon from "@mui/icons-material/Google";
import HomeTemplate from "../../components/templates/HomeTemplate";
import { useAppSelector } from "../../state";
import { selectIsUserPendingAuthentication } from "../../state/user";
import firestoreAuth from "../../services/firestore-auth";
import { useAppDispatch, useAppSelector } from "../../state";
import { login, selectIsUserPendingAuthentication } from "../../state/user";

export default function AuthMain() {
const handleLoginClick = () => firestoreAuth.redirectLogin();
const dispatch = useAppDispatch();

const handleLoginClick = () => dispatch(login());

const isPending = useAppSelector(selectIsUserPendingAuthentication);

return (
<HomeTemplate>
<Typography
Expand Down
9 changes: 5 additions & 4 deletions src/services/firestore-auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import * as firebaseAuth from "firebase/auth";
describe("firestoreAuth", () => {
it("should successfully log in and return user information", async () => {
jest.spyOn(firebaseAuth, "getAuth");
jest.spyOn(firebaseAuth, "signInWithPopup");
const user = await firestoreAuth.login();
jest.spyOn(firebaseAuth, "getRedirectResult");
await firestoreAuth.login();
const user = await firestoreAuth.loadUser();
expect(user).toEqual({
uid: "abc123def456",
displayName: "Jane Doe",
Expand All @@ -16,7 +17,7 @@ describe("firestoreAuth", () => {

it("should block log in when user email is null", async () => {
jest.spyOn(firebaseAuth, "getAuth");
jest.spyOn(firebaseAuth, "signInWithPopup").mockResolvedValue({
jest.spyOn(firebaseAuth, "getRedirectResult").mockResolvedValue({
operationType: "signIn",
providerId: "google.com",
user: {
Expand All @@ -39,7 +40,7 @@ describe("firestoreAuth", () => {
photoURL: "",
},
});
await expect(firestoreAuth.login()).rejects.toThrow(
await expect(firestoreAuth.loadUser()).rejects.toThrow(
"Login blocked: unidentified user."
);
});
Expand Down
77 changes: 33 additions & 44 deletions src/services/firestore-auth.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import once from "lodash.once";
import type { User as FirebaseUser, UserCredential } from "firebase/auth";
import type { User as FirebaseUser } from "firebase/auth";
import {
browserLocalPersistence,
getAuth,
Expand All @@ -14,67 +14,56 @@ import type { User } from "../webrtc";
const firestoreAuth = {
loadUser,
login,
redirectLogin,
logout,
};

export default firestoreAuth;

/** Loads user from redirection result, persistence, whatever. */
async function loadUser(): Promise<User> {
const empty: User = { uid: "", displayName: "", email: "" };

return new Promise((resolve) => {
const done = (user: FirebaseUser | null) => {
try {
if (user) {
const { uid, displayName, email } = user;
resolve({
uid,
displayName: displayName ?? email ?? "",
email: email ?? "?@?",
});
} else {
resolve(empty);
}
} catch (error) {
return new Promise((resolve, reject) => {
const handleResult = (user: FirebaseUser | null) => {
if (!user) {
const empty: User = { uid: "", displayName: "", email: "" };
resolve(empty);
return;
}

if (!user.uid || !user.email) {
reject(new Error("Login blocked: unidentified user."));
return;
}

resolve({
uid: user.uid,
displayName: user.displayName ?? user.email,
email: user.email,
});
};

setTimeout(() => done(null), 3000);
getAuth().onAuthStateChanged(once(done));
// prepare auth for one of the following events:
const auth = getAuth();
const onceHandleResult = once(handleResult);
// case of timeout (this was specially important for popup method but now not so much)
setTimeout(() => onceHandleResult(null), 5 * 1000);
// case of loaded by something else (like persistent successfully loaded)
auth.onAuthStateChanged(onceHandleResult);
// case of got result from redirect flow
getRedirectResult(auth)
.then((credentials) => credentials?.user || null)
.then(onceHandleResult);
});
}

async function redirectLogin(): Promise<never> {
/** Trigger the login function, it redirects to an authentication page. */
async function login(): Promise<void> {
const auth = getAuth();

await setPersistence(auth, browserLocalPersistence);
return signInWithRedirect(auth, googleAuthProvider);
}

async function login(): Promise<User> {
const empty: User = { uid: "", displayName: "", email: "" };
const auth = getAuth();
const result = await getRedirectResult(auth);

if (result === null) {
Promise.resolve(empty);
}
const {
user: { uid, displayName, email },
} = result as UserCredential;
if (!email) {
signOut(auth);
throw new Error("Login blocked: unidentified user.");
}
return {
uid,
displayName: displayName ?? email,
email,
};
await signInWithRedirect(auth, googleAuthProvider);
}

/** Clears the authentication data. */
async function logout() {
signOut(getAuth());
}
2 changes: 1 addition & 1 deletion src/state/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export const loadUser = createAsyncThunk(

export const login = createAsyncThunk(
"user/login",
() => firestoreAuth.login(),
() => firestoreAuth.login().then(firestoreAuth.loadUser),
{
condition: (_arg, thunkAPI) =>
(thunkAPI.getState() as RootState).user.status === "idle",
Expand Down
5 changes: 3 additions & 2 deletions src/testing-helpers/mock-firebase-auth.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Auth, AuthProvider } from "firebase/auth";

jest.mock("firebase/auth", () => ({
getAuth: jest.fn(),
getAuth: jest.fn().mockReturnValue({ onAuthStateChanged: jest.fn() }),
GoogleAuthProvider: jest.fn(() => ({
Qc: ["client_id", "response_type", "scope", "redirect_uri", "state"],
providerId: "google.com",
Expand All @@ -13,8 +13,9 @@ jest.mock("firebase/auth", () => ({
})),
setPersistence: jest.fn(),
browserLocalPersistence: jest.fn(),
signInWithRedirect: jest.fn(),
// eslint-disable-next-line @typescript-eslint/no-unused-vars
signInWithPopup: jest.fn((_: Auth, __: AuthProvider) =>
getRedirectResult: jest.fn((_: Auth, __: AuthProvider) =>
Promise.resolve({
operationType: "signIn",
providerId: "google.com",
Expand Down

0 comments on commit 5b8c762

Please sign in to comment.