From 7058f1c2d65367e9e7fc1a8707b10dc08615eca4 Mon Sep 17 00:00:00 2001 From: lenhanphung <44486647+lenhanphung@users.noreply.github.com> Date: Mon, 26 Jan 2026 17:13:26 +0700 Subject: [PATCH] Feat/update login flow reload page (#467) * update spacing event modal * update spacing create calendar modal * fix evenchip description color * using variant instead of sx * update login flow * fix: add useEffect import back to Calendar.tsx --- __test__/features/user/HandleLogin.test.tsx | 68 ++++++++--- __test__/features/user/LoginCallback.test.tsx | 81 ++++++++++-- src/App.tsx | 2 + src/app/loadingSlice.ts | 30 +++++ src/app/store.ts | 2 + src/components/Calendar/Calendar.tsx | 7 -- src/features/User/HandleLogin.tsx | 44 ++++--- src/features/User/LoginCallback.tsx | 115 +++++++++++++----- src/utils/apiUtils.ts | 15 ++- 9 files changed, 284 insertions(+), 80 deletions(-) create mode 100644 src/app/loadingSlice.ts diff --git a/__test__/features/user/HandleLogin.test.tsx b/__test__/features/user/HandleLogin.test.tsx index 9a3e2fa..4d5aa7b 100644 --- a/__test__/features/user/HandleLogin.test.tsx +++ b/__test__/features/user/HandleLogin.test.tsx @@ -28,21 +28,50 @@ describe("HandleLogin", () => { jest.spyOn(oidcAuth, "Auth").mockResolvedValue(loginUrlMock); - renderWithProviders(); - - await waitFor(() => { - expect(oidcAuth.Auth).toHaveBeenCalled(); - expect(sessionStorage.getItem("redirectState")).toEqual( - JSON.stringify({ - code_verifier: "verifier123", - state: "state123", - }) - ); - expect(apiUtils.redirectTo).toHaveBeenCalledWith(loginUrlMock.redirectTo); + renderWithProviders(, { + user: { + userData: null, + tokens: null, + loading: false, + error: null, + }, + calendars: { + list: {}, + pending: false, + error: null, + }, }); + + await waitFor( + () => { + expect(oidcAuth.Auth).toHaveBeenCalled(); + }, + { timeout: 3000 } + ); + + await waitFor( + () => { + expect(sessionStorage.getItem("redirectState")).toEqual( + JSON.stringify({ + code_verifier: "verifier123", + state: "state123", + }) + ); + }, + { timeout: 3000 } + ); + + await waitFor( + () => { + expect(apiUtils.redirectTo).toHaveBeenCalledWith( + loginUrlMock.redirectTo + ); + }, + { timeout: 3000 } + ); }); - test("shows Loading when userData exists and calendars pending is true", () => { + test("does not render loading element when userData exists and calendars pending is true", () => { const preloadedState = { user: { userData: { @@ -51,14 +80,18 @@ describe("HandleLogin", () => { sid: "aiYbWZSk2g0F+LrQeD7Dg4QcUMR8R/zTZdZBiA7N6Ro", openpaasId: "667037022b752d0026472254", }, + tokens: { access_token: "test" }, + loading: false, }, calendars: { list: {}, pending: true }, + loading: { isLoading: true }, }; renderWithProviders(, preloadedState); - expect(screen.getByTestId("loading")).toBeInTheDocument(); + // HandleLogin now returns null, loading is shown at App level via appLoading state + expect(screen.queryByTestId("loading")).not.toBeInTheDocument(); }); - test("shows Loading when userData exists and calendars pending is false", () => { + test("does not render loading element when userData exists and calendars pending is false", () => { const preloadedState = { user: { userData: { @@ -67,11 +100,16 @@ describe("HandleLogin", () => { sid: "aiYbWZSk2g0F+LrQeD7Dg4QcUMR8R/zTZdZBiA7N6Ro", openpaasId: "667037022b752d0026472254", }, + tokens: { access_token: "test" }, + loading: false, }, + calendars: { list: {}, pending: false }, + loading: { isLoading: false }, }; renderWithProviders(, preloadedState); - expect(screen.getByTestId("loading")).toBeInTheDocument(); + // HandleLogin now returns null, loading is shown at App level via appLoading state + expect(screen.queryByTestId("loading")).not.toBeInTheDocument(); }); test("goes to error page when there is error in user data", () => { const dispatch = appHooks.useAppDispatch(); diff --git a/__test__/features/user/LoginCallback.test.tsx b/__test__/features/user/LoginCallback.test.tsx index b3037d7..97f41a2 100644 --- a/__test__/features/user/LoginCallback.test.tsx +++ b/__test__/features/user/LoginCallback.test.tsx @@ -1,4 +1,4 @@ -import { useAppDispatch } from "@/app/hooks"; +import { useAppDispatch, useAppSelector } from "@/app/hooks"; import { getCalendarsListAsync } from "@/features/Calendars/services/getCalendarsListAsync"; import { CallbackResume } from "@/features/User/LoginCallback"; import * as oidcAuth from "@/features/User/oidcAuth"; @@ -8,13 +8,26 @@ import { setUserData, } from "@/features/User/userSlice"; import { render, waitFor } from "@testing-library/react"; -import { push } from "redux-first-history"; +import { replace } from "redux-first-history"; import { renderWithProviders } from "../../utils/Renderwithproviders"; +import { setAppLoading } from "@/app/loadingSlice"; // Mocks jest.mock("@/app/hooks", () => ({ useAppDispatch: jest.fn(), - useAppSelector: jest.fn(() => ({})), + useAppSelector: jest.fn(() => ({ + user: { + userData: null, + tokens: null, + loading: false, + error: null, + }, + calendars: { + list: {}, + pending: false, + error: null, + }, + })), })); jest.mock("@/features/User/oidcAuth", () => ({ @@ -55,10 +68,33 @@ jest.mock("@/features/Calendars/services/getCalendarsListAsync", () => { describe("CallbackResume", () => { const dispatch = jest.fn(); + let mockUserState: any; + let mockCalendarsState: any; beforeEach(() => { jest.clearAllMocks(); (useAppDispatch as unknown as jest.Mock).mockReturnValue(dispatch); + + // Initialize mock states + mockUserState = { + userData: null, + tokens: null, + loading: false, + error: null, + }; + mockCalendarsState = { + list: {}, + pending: false, + error: null, + }; + + (useAppSelector as jest.Mock).mockImplementation((selector) => { + const state = { + user: mockUserState, + calendars: mockCalendarsState, + }; + return selector(state); + }); }); it("should call Callback and dispatch necessary actions", async () => { @@ -77,11 +113,14 @@ describe("CallbackResume", () => { JSON.stringify({ code_verifier: "verifier123", state: "state456" }) ); - render(); + const { rerender } = render(); await waitFor(() => { expect(oidcAuth.Callback).toHaveBeenCalledWith("verifier123", "state456"); }); + await waitFor(() => { + expect(dispatch).toHaveBeenCalledWith(setAppLoading(true)); + }); await waitFor(() => { expect(dispatch).toHaveBeenCalledWith(setUserData(mockUserInfo)); }); @@ -94,9 +133,35 @@ describe("CallbackResume", () => { await waitFor(() => { expect(dispatch).toHaveBeenCalledWith(getCalendarsListAsync()); }); - await waitFor(() => { - expect(dispatch).toHaveBeenCalledWith(push("/")); - }); + + // Simulate async actions completing by updating mock state + mockUserState = { + userData: mockUserInfo, + tokens: mockTokenSet, + loading: false, + error: null, + }; + mockCalendarsState = { + list: { calendar1: {} }, + pending: false, + error: null, + }; + + // Re-render to trigger navigation effect + rerender(); + + await waitFor( + () => { + expect(dispatch).toHaveBeenCalledWith(setAppLoading(false)); + }, + { timeout: 3000 } + ); + await waitFor( + () => { + expect(dispatch).toHaveBeenCalledWith(replace("/calendar")); + }, + { timeout: 3000 } + ); await waitFor(() => { expect(sessionStorage.getItem("redirectState")).toBe(null); }); @@ -112,7 +177,7 @@ describe("CallbackResume", () => { renderWithProviders(); await waitFor(() => { - expect(dispatch).toHaveBeenCalledWith(push("/")); + expect(dispatch).toHaveBeenCalledWith(replace("/")); }); }); }); diff --git a/src/App.tsx b/src/App.tsx index 5d66f3d..adb25b7 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -42,6 +42,7 @@ const isValidLanguage = ( function App() { const error = useAppSelector((state) => state.user.error); + const appLoading = useAppSelector((state) => state.loading.isLoading); const userLanguage = useAppSelector( (state) => state.user.coreConfig.language ); @@ -80,6 +81,7 @@ function App() { + {appLoading && } ); diff --git a/src/app/loadingSlice.ts b/src/app/loadingSlice.ts new file mode 100644 index 0000000..6eb9a52 --- /dev/null +++ b/src/app/loadingSlice.ts @@ -0,0 +1,30 @@ +import { createSlice, PayloadAction } from "@reduxjs/toolkit"; + +interface LoadingState { + isLoading: boolean; + message?: string; +} + +const initialState: LoadingState = { + isLoading: false, + message: undefined, +}; + +const loadingSlice = createSlice({ + name: "loading", + initialState, + reducers: { + setAppLoading: (state, action: PayloadAction) => { + state.isLoading = action.payload; + if (!action.payload) { + state.message = undefined; + } + }, + setLoadingMessage: (state, action: PayloadAction) => { + state.message = action.payload; + }, + }, +}); + +export const { setAppLoading, setLoadingMessage } = loadingSlice.actions; +export default loadingSlice.reducer; diff --git a/src/app/store.ts b/src/app/store.ts index bf059b5..595852c 100644 --- a/src/app/store.ts +++ b/src/app/store.ts @@ -3,6 +3,7 @@ import searchResultReducer from "@/features/Search/SearchSlice"; import settingsReducer from "@/features/Settings/SettingsSlice"; import userReducer from "@/features/User/userSlice"; import { combineReducers, configureStore } from "@reduxjs/toolkit"; +import loadingReducer from "./loadingSlice"; import { createBrowserHistory } from "history"; import { createReduxHistoryContext } from "redux-first-history"; @@ -15,6 +16,7 @@ const rootReducer = combineReducers({ calendars: eventsCalendar, settings: settingsReducer, searchResult: searchResultReducer, + loading: loadingReducer, }); export const setupStore = (preloadedState?: Partial) => { diff --git a/src/components/Calendar/Calendar.tsx b/src/components/Calendar/Calendar.tsx index 44e0a16..ea3324d 100644 --- a/src/components/Calendar/Calendar.tsx +++ b/src/components/Calendar/Calendar.tsx @@ -26,7 +26,6 @@ import timeGridPlugin from "@fullcalendar/timegrid"; import { Box, Button, radius, useTheme } from "@linagora/twake-mui"; import AddIcon from "@mui/icons-material/Add"; import { MutableRefObject, useEffect, useMemo, useRef, useState } from "react"; -import { push } from "redux-first-history"; import { useI18n } from "twake-i18n"; import { User } from "../Attendees/PeopleSearch"; import { EventErrorSnackbar } from "../Error/ErrorSnackbar"; @@ -67,16 +66,10 @@ export default function CalendarApp({ }: CalendarAppProps) { const [selectedDate, setSelectedDate] = useState(new Date()); const [selectedMiniDate, setSelectedMiniDate] = useState(new Date()); - const tokens = useAppSelector((state) => state.user.tokens); const userId = useAppSelector((state) => state.user.userData?.openpaasId) ?? ""; const dispatch = useAppDispatch(); const theme = useTheme(); - useEffect(() => { - if (!tokens || !userId) { - dispatch(push("/")); - } - }, [dispatch, tokens, userId]); const view = useAppSelector((state) => state.settings.view); const userData = useAppSelector((state) => state.user.userData); const hideDeclinedEvents = useAppSelector( diff --git a/src/features/User/HandleLogin.tsx b/src/features/User/HandleLogin.tsx index 923d93e..41308ec 100644 --- a/src/features/User/HandleLogin.tsx +++ b/src/features/User/HandleLogin.tsx @@ -1,26 +1,26 @@ import { useAppDispatch, useAppSelector } from "@/app/hooks"; -import { Loading } from "@/components/Loading/Loading"; import { redirectTo } from "@/utils/apiUtils"; import { useEffect, useRef } from "react"; import { push } from "redux-first-history"; import { getCalendarsListAsync } from "../Calendars/services"; import { Auth } from "./oidcAuth"; import { getOpenPaasUserDataAsync, setTokens, setUserData } from "./userSlice"; +import { setAppLoading } from "@/app/loadingSlice"; export function HandleLogin() { const userData = useAppSelector((state) => state.user); const calendars = useAppSelector((state) => state.calendars); const dispatch = useAppDispatch(); const hasInitiatedRef = useRef(false); - const calendarsLoadingRef = useRef(false); + const hasNavigatedRef = useRef(false); + // Initiate login or load saved data useEffect(() => { if (hasInitiatedRef.current) return; - if (userData.userData) return; - if (Object.keys(calendars.list).length > 0) return; - if (calendarsLoadingRef.current) return; + if (userData.userData && !calendars.pending) return; hasInitiatedRef.current = true; + const initiateLogin = async () => { const savedToken = sessionStorage.getItem("tokenSet") ? JSON.parse(sessionStorage.getItem("tokenSet")!) @@ -30,13 +30,11 @@ export function HandleLogin() { : null; if (savedToken && savedUser) { + dispatch(setAppLoading(true)); dispatch(setTokens(savedToken)); dispatch(setUserData(savedUser)); await dispatch(getOpenPaasUserDataAsync()); - calendarsLoadingRef.current = true; await dispatch(getCalendarsListAsync()); - calendarsLoadingRef.current = false; - dispatch(push("/calendar")); return; } @@ -55,26 +53,34 @@ export function HandleLogin() { initiateLogin(); }, [userData.userData, calendars.list, dispatch]); + + // Navigate to /calendar only when all data is ready useEffect(() => { - if (userData.error) { + if (hasNavigatedRef.current) return; + if (userData.loading || calendars.pending) return; + if (userData.error || calendars.error) { + dispatch(setAppLoading(false)); dispatch(push("/error")); + return; } - if ( - !calendars.pending && - !userData.loading && - !userData.error && - !calendars.error - ) { - dispatch(push("/calendar")); - } + if (!userData.userData || !userData.tokens) return; + // Calendars list can be empty, that's valid - just need to finish loading + + // All data is ready, navigate to calendar + hasNavigatedRef.current = true; + dispatch(setAppLoading(false)); + dispatch(push("/calendar")); }, [ - calendars.pending, userData.loading, + userData.userData, + userData.tokens, userData.error, + calendars.pending, calendars.error, dispatch, ]); - return ; + + return null; } export default HandleLogin; diff --git a/src/features/User/LoginCallback.tsx b/src/features/User/LoginCallback.tsx index 49dc2ff..6ecdea9 100644 --- a/src/features/User/LoginCallback.tsx +++ b/src/features/User/LoginCallback.tsx @@ -1,7 +1,6 @@ -import { useAppDispatch } from "@/app/hooks"; -import { Loading } from "@/components/Loading/Loading"; +import { useAppDispatch, useAppSelector } from "@/app/hooks"; import { useEffect, useRef } from "react"; -import { push } from "redux-first-history"; +import { replace } from "redux-first-history"; import { getCalendarsListAsync } from "../Calendars/services"; import { Callback } from "./oidcAuth"; import { @@ -10,57 +9,113 @@ import { setUserData, setUserError, } from "./userSlice"; +import { setAppLoading } from "@/app/loadingSlice"; export function CallbackResume() { const dispatch = useAppDispatch(); const hasRun = useRef(false); - const calendarsLoadingRef = useRef(false); - const saved = sessionStorage.getItem("redirectState") - ? JSON.parse(sessionStorage.getItem("redirectState")!) - : null; + const hasNavigated = useRef(false); + const userData = useAppSelector((state) => state.user); + const calendars = useAppSelector((state) => state.calendars); + + // Process callback and load data useEffect(() => { if (hasRun.current) { return; } hasRun.current = true; + const runCallback = async () => { + // Read redirectState inside useEffect to avoid stale closures + const saved = sessionStorage.getItem("redirectState") + ? JSON.parse(sessionStorage.getItem("redirectState")!) + : null; + + // Check if we have saved tokens (already logged in) + const savedToken = sessionStorage.getItem("tokenSet") + ? JSON.parse(sessionStorage.getItem("tokenSet")!) + : null; + + // If no redirectState but we have saved session, just go home + // This can happen if user refreshes callback page or gets redirected here after already logged in + if (!saved?.code_verifier) { + if (savedToken) { + sessionStorage.removeItem("redirectState"); + dispatch(replace("/")); + return; + } + + console.warn("Missing redirectState"); + sessionStorage.removeItem("redirectState"); + dispatch(replace("/")); + return; + } + try { - const data = await Callback(saved?.code_verifier, saved?.state); + dispatch(setAppLoading(true)); + + const data = await Callback(saved.code_verifier, saved.state); + if (!data || !data.userinfo || !data.tokenSet) { throw new Error("OAuth callback failed"); } - dispatch(setUserData(data?.userinfo)); - dispatch(setTokens(data?.tokenSet)); + + // IMPORTANT: Save tokens to sessionStorage FIRST before making any API calls + // because API calls will read token from sessionStorage + sessionStorage.setItem("tokenSet", JSON.stringify(data.tokenSet)); + sessionStorage.setItem("userData", JSON.stringify(data.userinfo)); + + dispatch(setUserData(data.userinfo)); + dispatch(setTokens(data.tokenSet)); + await dispatch(getOpenPaasUserDataAsync()); - if (!calendarsLoadingRef.current) { - calendarsLoadingRef.current = true; - await dispatch(getCalendarsListAsync()); - calendarsLoadingRef.current = false; - } + await dispatch(getCalendarsListAsync()); sessionStorage.removeItem("redirectState"); - sessionStorage.setItem("tokenSet", JSON.stringify(data?.tokenSet)); - sessionStorage.setItem("userData", JSON.stringify(data?.userinfo)); - // Redirect to main page after successful callback - dispatch(push("/")); } catch (e) { console.error("OIDC callback error:", e); - // Redirect to error page after error + dispatch(setAppLoading(false)); dispatch( setUserError(e instanceof Error ? e.message : "OAuth callback failed") ); - dispatch(push("/error")); + dispatch(replace("/error")); } }; - if (saved?.code_verifier) { - runCallback(); - } else { - console.warn("Missing redirectState"); - sessionStorage.removeItem("redirectState"); - dispatch(push("/")); - } - }, [dispatch, saved]); + runCallback(); + }, [dispatch]); - return ; + // Navigate to /calendar only when all data is ready + useEffect(() => { + if (hasNavigated.current) return; + if (userData.loading || calendars.pending) return; + if (userData.error || calendars.error) { + dispatch(setAppLoading(false)); + dispatch(replace("/error")); + return; + } + if (!userData.userData || !userData.tokens) return; + // Calendars list can be empty, that's valid - just need to finish loading + + // All data is ready, navigate to calendar + hasNavigated.current = true; + dispatch(setAppLoading(false)); + + // Clear any query params from URL first, then navigate + if (window.location.search) { + window.history.replaceState({}, "", window.location.pathname); + } + + dispatch(replace("/calendar")); + }, [ + userData.loading, + userData.userData, + userData.tokens, + userData.error, + calendars.pending, + calendars.error, + dispatch, + ]); + + return null; } diff --git a/src/utils/apiUtils.ts b/src/utils/apiUtils.ts index 98efaf0..c00536b 100644 --- a/src/utils/apiUtils.ts +++ b/src/utils/apiUtils.ts @@ -50,7 +50,19 @@ export const api = ky.extend({ afterResponse: [ async (request, options, response) => { if (response.status === 401) { - // Attempt token refresh on unauthorized response + // Check if we're already on login flow to prevent redirect loop + const currentPath = window.location.pathname; + if (currentPath === "/" || currentPath === "/callback") { + return response; + } + + // Check if we have a token in the request + const hasAuthHeader = request.headers.has("Authorization"); + if (!hasAuthHeader) { + return response; + } + + // Only redirect to SSO if we're sure token is invalid (not just missing) const loginurl = await Auth(); sessionStorage.setItem( @@ -62,6 +74,7 @@ export const api = ky.extend({ ); redirectTo(loginurl.redirectTo); } + return response; }, ], },