From d60ea1acd45e30ddb85e0996dcc24c7bb27e7ff7 Mon Sep 17 00:00:00 2001 From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Date: Thu, 17 Apr 2025 12:07:08 +0200 Subject: [PATCH] feat: redirect logic refactor (#9734) --- .github/workflows/e2e.frontend.yaml | 1 + frontend/cypress/global.d.ts | 1 + .../cypress/integration/login/login.spec.ts | 80 +++++++++++++++++++ frontend/cypress/support/UI.ts | 43 +++++----- frontend/cypress/support/commands.ts | 2 + frontend/src/component/App.tsx | 7 +- frontend/src/component/InitialRedirect.tsx | 74 +++++++++++------ .../FeatureToggleListTable.tsx | 6 +- .../project/Project/Project.styles.ts | 2 +- .../component/user/SimpleAuth/SimpleAuth.tsx | 8 +- .../api/getters/useAuth/useAuthEndpoint.ts | 7 +- frontend/src/hooks/useLocalStorageState.ts | 3 +- frontend/src/utils/createLocalStorage.ts | 5 +- frontend/src/utils/storage.ts | 4 +- 14 files changed, 185 insertions(+), 58 deletions(-) create mode 100644 frontend/cypress/integration/login/login.spec.ts diff --git a/.github/workflows/e2e.frontend.yaml b/.github/workflows/e2e.frontend.yaml index 9137e3ea6c..d37e8ca808 100644 --- a/.github/workflows/e2e.frontend.yaml +++ b/.github/workflows/e2e.frontend.yaml @@ -13,6 +13,7 @@ jobs: - groups/groups.spec.ts - projects/access.spec.ts - segments/segments.spec.ts + - login/login.spec.ts steps: - name: Dump GitHub context env: diff --git a/frontend/cypress/global.d.ts b/frontend/cypress/global.d.ts index eeb25b91a9..e3162a2cca 100644 --- a/frontend/cypress/global.d.ts +++ b/frontend/cypress/global.d.ts @@ -21,6 +21,7 @@ declare namespace Cypress { interface Chainable { runBefore(): Chainable; + do_login(user = AUTH_USER, password = AUTH_PASSWORD): Chainable; login_UI(user = AUTH_USER, password = AUTH_PASSWORD): Chainable; logout_UI(): Chainable; diff --git a/frontend/cypress/integration/login/login.spec.ts b/frontend/cypress/integration/login/login.spec.ts new file mode 100644 index 0000000000..7f6ed3b1c1 --- /dev/null +++ b/frontend/cypress/integration/login/login.spec.ts @@ -0,0 +1,80 @@ +/// + +describe('login', { testIsolation: true }, () => { + const baseUrl = Cypress.config().baseUrl; + const randomId = String(Math.random()).split('.')[1]; + const projectName = `unleash-e2e-login-project-${randomId}`; + + before(() => { + cy.runBefore(); + Cypress.session.clearAllSavedSessions(); + cy.login_UI(); + cy.createProject_API(projectName); + cy.logout_UI(); + }); + + after(() => { + cy.deleteProject_API(projectName); + }); + + beforeEach(() => { + cy.login_UI(); + }); + + it('is redirecting to /personal after first login', () => { + cy.visit('/'); + cy.url().should('eq', `${baseUrl}/personal`); + cy.visit('/'); + // "/" should again redirect to last "home" page + cy.url().should('eq', `${baseUrl}/personal`); + }); + + it('is redirecting to last visited projects', () => { + cy.visit('/projects'); + cy.visit('/'); + cy.url().should((url) => url.startsWith(`${baseUrl}/projects`)); + cy.contains('a', projectName).click(); + cy.get(`h1 span`).should('not.have.class', 'skeleton'); + cy.visit('/'); + cy.url().should((url) => + // last visited project + url.startsWith(`${baseUrl}/projects/${projectName}`), + ); + }); + + it('is redirecting to other pages', () => { + cy.visit('/search'); + cy.visit('/playground'); + cy.visit('/'); + cy.url().should('eq', `${baseUrl}/playground`); + cy.visit('/admin'); + cy.visit('/applications'); // not one of main pages + cy.visit('/'); + cy.url().should('eq', `${baseUrl}/admin`); + }); + + it('clears last visited page on manual logout', () => { + cy.visit('/search'); + cy.get('[data-testid=HEADER_USER_AVATAR]').click(); + cy.get('button').contains('Log out').click(); + cy.url().should('eq', `${baseUrl}/login`); + cy.visit('/'); + cy.url().should('eq', `${baseUrl}/login`); + cy.do_login(); + cy.visit('/'); + cy.url().should('eq', `${baseUrl}/personal`); + }); + + it('remembers last visited page on next login', () => { + cy.visit('/insights'); + cy.window().then((win) => { + win.sessionStorage.clear(); // not localStorage + win.location.reload(); + }); + cy.visit('/'); + cy.url().should('eq', `${baseUrl}/login`); + cy.do_login(); + cy.visit('/'); + cy.url().should('eq', `${baseUrl}/insights`); + }); +}); diff --git a/frontend/cypress/support/UI.ts b/frontend/cypress/support/UI.ts index e854ef234e..66b11ce76c 100644 --- a/frontend/cypress/support/UI.ts +++ b/frontend/cypress/support/UI.ts @@ -24,28 +24,35 @@ export const runBefore = () => { disableActiveSplashScreens(); }; +export const do_login = ( + user = AUTH_USER, + password = AUTH_PASSWORD, +): Chainable => { + cy.visit('/'); + cy.wait(200); + cy.get("[data-testid='LOGIN_EMAIL_ID']").type(user); + + if (AUTH_PASSWORD) { + cy.get("[data-testid='LOGIN_PASSWORD_ID']").type(password); + } + + cy.get("[data-testid='LOGIN_BUTTON']").click(); + + // Wait for the login redirect to complete. + cy.get("[data-testid='HEADER_USER_AVATAR']"); + + if (document.querySelector("[data-testid='CLOSE_SPLASH']")) { + cy.get("[data-testid='CLOSE_SPLASH']").click(); + } + + return cy; +}; + export const login_UI = ( user = AUTH_USER, password = AUTH_PASSWORD, ): Chainable => { - return cy.session(user, () => { - cy.visit('/'); - cy.wait(200); - cy.get("[data-testid='LOGIN_EMAIL_ID']").type(user); - - if (AUTH_PASSWORD) { - cy.get("[data-testid='LOGIN_PASSWORD_ID']").type(password); - } - - cy.get("[data-testid='LOGIN_BUTTON']").click(); - - // Wait for the login redirect to complete. - cy.get("[data-testid='HEADER_USER_AVATAR']"); - - if (document.querySelector("[data-testid='CLOSE_SPLASH']")) { - cy.get("[data-testid='CLOSE_SPLASH']").click(); - } - }); + return cy.session(user, () => do_login(user, password)); }; export const createFeature_UI = ( diff --git a/frontend/cypress/support/commands.ts b/frontend/cypress/support/commands.ts index 1674fa9ced..3183303723 100644 --- a/frontend/cypress/support/commands.ts +++ b/frontend/cypress/support/commands.ts @@ -12,6 +12,7 @@ import { addFlexibleRolloutStrategyToFeature_UI, addUserIdStrategyToFeature_UI, updateFlexibleRolloutStrategy_UI, + do_login, //@ts-ignore } from './UI'; import { @@ -32,6 +33,7 @@ Cypress.on('window:before:load', (window) => { }); Cypress.Commands.add('runBefore', runBefore); Cypress.Commands.add('login_UI', login_UI); +Cypress.Commands.add('do_login', do_login); Cypress.Commands.add('createSegment_UI', createSegment_UI); Cypress.Commands.add('deleteSegment_UI', deleteSegment_UI); Cypress.Commands.add('deleteFeature_API', deleteFeature_API); diff --git a/frontend/src/component/App.tsx b/frontend/src/component/App.tsx index 5f23afba35..813b728efd 100644 --- a/frontend/src/component/App.tsx +++ b/frontend/src/component/App.tsx @@ -1,5 +1,5 @@ import { Suspense, useEffect } from 'react'; -import { Route, Routes } from 'react-router-dom'; +import { Route, Routes, useLocation } from 'react-router-dom'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import { FeedbackNPS } from 'component/feedback/FeedbackNPS/FeedbackNPS'; import { LayoutPicker } from 'component/layout/LayoutPicker/LayoutPicker'; @@ -16,7 +16,7 @@ import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig'; import MaintenanceBanner from './maintenance/MaintenanceBanner'; import { styled } from '@mui/material'; -import { InitialRedirect } from './InitialRedirect'; +import { InitialRedirect, useLastViewedPage } from './InitialRedirect'; import { InternalBanners } from './banners/internalBanners/InternalBanners'; import { ExternalBanners } from './banners/externalBanners/ExternalBanners'; import { LicenseBanner } from './banners/internalBanners/LicenseBanner'; @@ -51,6 +51,9 @@ export const App = () => { const isLoggedIn = Boolean(user?.id); + const location = useLocation(); + useLastViewedPage(location); + return ( }> diff --git a/frontend/src/component/InitialRedirect.tsx b/frontend/src/component/InitialRedirect.tsx index 2ac97d278d..5c0ebd201a 100644 --- a/frontend/src/component/InitialRedirect.tsx +++ b/frontend/src/component/InitialRedirect.tsx @@ -1,37 +1,59 @@ -import { useCallback, useEffect } from 'react'; -import { useNavigate } from 'react-router-dom'; +import { useEffect } from 'react'; +import { type Location, Navigate } from 'react-router-dom'; import useProjects from 'hooks/api/getters/useProjects/useProjects'; import { useLastViewedProject } from 'hooks/useLastViewedProject'; import Loader from './common/Loader/Loader'; -import { getSessionStorageItem, setSessionStorageItem } from 'utils/storage'; +import { useLocalStorageState } from 'hooks/useLocalStorageState'; +import { useAuthUser } from 'hooks/api/getters/useAuth/useAuthUser'; -export const InitialRedirect = () => { - const { lastViewed } = useLastViewedProject(); - const { projects, loading } = useProjects(); - const navigate = useNavigate(); - const sessionRedirect = getSessionStorageItem('login-redirect'); - - // Redirect based on project and last viewed - const getRedirect = useCallback(() => { - if (projects && lastViewed) { - return `/projects/${lastViewed}`; - } - - return '/personal'; - }, [lastViewed, projects]); - - const redirect = () => { - navigate(sessionRedirect ?? getRedirect(), { replace: true }); - }; +export const useLastViewedPage = (location?: Location) => { + const [state, setState] = useLocalStorageState( + 'lastViewedPage', + '/personal', + 7 * 24 * 60 * 60 * 1000, // 7 days, left to promote seeing Personal dashboard from time to time + ); useEffect(() => { - setSessionStorageItem('login-redirect'); - redirect(); - }, [getRedirect]); + if (location) { + const page = [ + '/personal', + '/projects', + '/search', + '/playground', + '/insights', + '/admin', + ].find( + (page) => + page === location.pathname || + location.pathname.startsWith(`/{page}/`), + ); + if (page) { + setState(page); + } + } + }, [location]); - if (loading) { + return state; +}; + +export const InitialRedirect = () => { + const { user, loading: isLoadingAuth } = useAuthUser(); + const { loading: isLoadingProjects } = useProjects(); + const isLoggedIn = Boolean(user?.id); + const lastViewedPage = useLastViewedPage(); + const { lastViewed: lastViewedProject } = useLastViewedProject(); + + if (isLoadingAuth || isLoadingProjects) { return ; } - return null; + if (!isLoggedIn) { + return ; + } + + if (lastViewedPage === '/projects' && lastViewedProject) { + return ; + } + + return ; }; diff --git a/frontend/src/component/feature/FeatureToggleList/FeatureToggleListTable.tsx b/frontend/src/component/feature/FeatureToggleList/FeatureToggleListTable.tsx index 7de1079cfd..254e5218ff 100644 --- a/frontend/src/component/feature/FeatureToggleList/FeatureToggleListTable.tsx +++ b/frontend/src/component/feature/FeatureToggleList/FeatureToggleListTable.tsx @@ -401,7 +401,11 @@ export const FeatureToggleListTable: FC = () => { bodyClass='no-padding' header={ ({ alignItems: 'start', })); -export const StyledProjectTitle = styled('span')(({ theme }) => ({ +export const StyledProjectTitle = styled('h1')(({ theme }) => ({ margin: 0, width: '100%', fontSize: theme.typography.h1.fontSize, diff --git a/frontend/src/component/user/SimpleAuth/SimpleAuth.tsx b/frontend/src/component/user/SimpleAuth/SimpleAuth.tsx index a0c2f8911d..cbc2f505b9 100644 --- a/frontend/src/component/user/SimpleAuth/SimpleAuth.tsx +++ b/frontend/src/component/user/SimpleAuth/SimpleAuth.tsx @@ -21,21 +21,24 @@ interface ISimpleAuthProps { const SimpleAuth: VFC = ({ authDetails, redirect }) => { const [email, setEmail] = useState(''); + const [isPending, setIsPending] = useState(false); const { refetchUser } = useAuthUser(); const { emailAuth } = useAuthApi(); const navigate = useNavigate(); const { setToastApiError } = useToast(); const handleSubmit: FormEventHandler = async (evt) => { + setIsPending(true); evt.preventDefault(); try { await emailAuth(authDetails.path, email); - refetchUser(); - navigate(redirect, { replace: true }); + await refetchUser(); + navigate(redirect); } catch (error) { setToastApiError(formatUnknownError(error)); } + setIsPending(false); }; const handleChange: ChangeEventHandler = (e) => { @@ -81,6 +84,7 @@ const SimpleAuth: VFC = ({ authDetails, redirect }) => { color='primary' className={styles.button} data-testid={LOGIN_BUTTON} + disabled={isPending} > Sign in diff --git a/frontend/src/hooks/api/getters/useAuth/useAuthEndpoint.ts b/frontend/src/hooks/api/getters/useAuth/useAuthEndpoint.ts index 964d2a6d43..38b3e0270d 100644 --- a/frontend/src/hooks/api/getters/useAuth/useAuthEndpoint.ts +++ b/frontend/src/hooks/api/getters/useAuth/useAuthEndpoint.ts @@ -58,9 +58,10 @@ export const useAuthEndpoint = (): IUseAuthEndpointOutput => { swrConfig, ); - const refetchAuth = useCallback(() => { - mutate(USER_ENDPOINT_PATH).catch(console.warn); - }, []); + const refetchAuth = useCallback( + async () => mutate(USER_ENDPOINT_PATH).catch(console.warn), + [], + ); return { data, diff --git a/frontend/src/hooks/useLocalStorageState.ts b/frontend/src/hooks/useLocalStorageState.ts index 8e3dd4dedc..3846bf6395 100644 --- a/frontend/src/hooks/useLocalStorageState.ts +++ b/frontend/src/hooks/useLocalStorageState.ts @@ -4,9 +4,10 @@ import { createLocalStorage } from '../utils/createLocalStorage'; export const useLocalStorageState = ( key: string, initialValue: T, + timeToLive?: number, ) => { const { value: initialStoredValue, setValue: setStoredValue } = - createLocalStorage(key, initialValue); + createLocalStorage(key, initialValue, timeToLive); const [localValue, setLocalValue] = useState(initialStoredValue); diff --git a/frontend/src/utils/createLocalStorage.ts b/frontend/src/utils/createLocalStorage.ts index f1d9e11562..2be6be4757 100644 --- a/frontend/src/utils/createLocalStorage.ts +++ b/frontend/src/utils/createLocalStorage.ts @@ -4,6 +4,7 @@ import { getLocalStorageItem, setLocalStorageItem } from './storage'; export const createLocalStorage = ( key: string, initialValue: T, + timeToLive?: number, ) => { const internalKey = `${basePath}:${key}:localStorage:v2`; const value = (() => { @@ -18,11 +19,11 @@ export const createLocalStorage = ( if (newValue instanceof Function) { const previousValue = getLocalStorageItem(internalKey); const output = newValue(previousValue ?? initialValue); - setLocalStorageItem(internalKey, output); + setLocalStorageItem(internalKey, output, timeToLive); return output; } - setLocalStorageItem(internalKey, newValue); + setLocalStorageItem(internalKey, newValue, timeToLive); return newValue; }; diff --git a/frontend/src/utils/storage.ts b/frontend/src/utils/storage.ts index c5873fd4fa..0a15869888 100644 --- a/frontend/src/utils/storage.ts +++ b/frontend/src/utils/storage.ts @@ -60,7 +60,7 @@ export function getLocalStorageItem(key: string): T | undefined { export function setLocalStorageItem( key: string, value: T | undefined = undefined, - timeToLive?: number, + timeToLive?: number, // milliseconds ) { try { const item: Expirable = { @@ -80,7 +80,7 @@ export function setLocalStorageItem( export function setSessionStorageItem( key: string, value: T | undefined = undefined, - timeToLive?: number, + timeToLive?: number, // milliseconds ) { try { const item: Expirable = {