From ac3a31e920f26b2e07572c3c583b06c3ead978b5 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Sun, 14 Dec 2025 06:59:24 +0100 Subject: [PATCH] fix: redirect to api /signin from client /settings (#64398) --- .../client-only-routes/show-settings.test.tsx | 91 +++++++++---------- .../src/client-only-routes/show-settings.tsx | 25 ++--- client/src/redux/create-store.ts | 1 - client/src/redux/hard-go-to-epic.js | 5 +- client/src/redux/index.js | 3 +- client/src/redux/root-epic.ts | 1 - client/src/redux/settings/danger-zone-saga.js | 9 +- 7 files changed, 61 insertions(+), 74 deletions(-) diff --git a/client/src/client-only-routes/show-settings.test.tsx b/client/src/client-only-routes/show-settings.test.tsx index 71d390681f0..7ba0a518ff7 100644 --- a/client/src/client-only-routes/show-settings.test.tsx +++ b/client/src/client-only-routes/show-settings.test.tsx @@ -1,60 +1,55 @@ -/* eslint-disable */ -// @ts-nocheck Likely need to not use ShallowRenderer import React from 'react'; -import ShallowRenderer from 'react-test-renderer/shallow'; -import { describe, it, expect, vi } from 'vitest'; +import { render } from '@testing-library/react'; +import { describe, it, expect, vi, beforeAll } from 'vitest'; +import { Provider } from 'react-redux'; import envData from '../../config/env.json'; -import { ShowSettings } from './show-settings'; +import ShowSettings from './show-settings'; -vi.mock('../analytics'); -vi.mock('@growthbook/growthbook-react', () => ({ - useFeatureIsOn: () => false -})); +import { createStore } from '../redux/create-store'; +import { initialState } from '../redux'; -const { apiLocation } = envData as Record; +vi.mock('../utils/get-words'); + +const { apiLocation } = envData; describe('', () => { - it('renders to the DOM when user is logged in', () => { - const shallow = new ShallowRenderer(); - shallow.render(); - expect(navigate).toHaveBeenCalledTimes(0); - const result = shallow.getRenderOutput(); - expect(result.type.toString()).toBe('Symbol(react.fragment)'); - // Renders Helmet component rather than Loader - expect(result.props.children[0].props.title).toEqual( - 'buttons.settings | freeCodeCamp.org' + beforeAll(() => { + // Location is not writable normally, so we have to delete and recreate + // https://github.com/jestjs/jest/issues/890#issuecomment-682286025 + const location = window.location as string & Location; + // @ts-expect-error TS is warning us that this breaks the type of + // window.location, since it is not optional, but we are replacing it with + // an object of the same type, it is safe to ignore. + delete global.window.location; + global.window.location = Object.assign({}, location); + }); + + it('does not navigate if already signed in', () => { + const store = createStore({ + app: { ...initialState, user: { sessionUser: 'anything truthy' } } + }); + const spy = vi.spyOn(window.location, 'href', 'set'); + + render( + + + ); + expect(spy).toHaveBeenCalledTimes(0); }); it('redirects to sign in page when user is not logged in', () => { - const shallow = new ShallowRenderer(); - shallow.render(); - expect(navigate).toHaveBeenCalledTimes(1); - expect(navigate).toHaveBeenCalledWith(`${apiLocation}/signin`); - const result = shallow.getRenderOutput(); - // Renders Loader rather than ShowSettings - expect(result.type.displayName).toBe('Loader'); + const store = createStore({ + app: { ...initialState, user: { sessionUser: null } } + }); + const spy = vi.spyOn(window.location, 'href', 'set'); + + render( + + + + ); + expect(spy).toHaveBeenCalledTimes(1); + expect(spy).toHaveBeenCalledWith(`${apiLocation}/signin`); }); }); - -const navigate = vi.fn(); -const loggedInProps = { - createFlashMessage: vi.fn(), - hardGoTo: vi.fn(), - isSignedIn: true, - navigate: navigate, - showLoading: false, - submitNewAbout: vi.fn(), - toggleTheme: vi.fn(), - updateSocials: vi.fn(), - updateIsHonest: vi.fn(), - updatePortfolio: vi.fn(), - updateQuincyEmail: vi.fn(), - user: { - about: '', - completedChallenges: [] - }, - verifyCert: vi.fn() -}; -const loggedOutProps = { ...loggedInProps }; -loggedOutProps.isSignedIn = false; diff --git a/client/src/client-only-routes/show-settings.tsx b/client/src/client-only-routes/show-settings.tsx index ed18d19ea89..bdfd4aa0b6f 100644 --- a/client/src/client-only-routes/show-settings.tsx +++ b/client/src/client-only-routes/show-settings.tsx @@ -1,4 +1,4 @@ -import React, { useRef, useEffect } from 'react'; +import React, { useEffect } from 'react'; import Helmet from 'react-helmet'; import { useTranslation } from 'react-i18next'; import { connect } from 'react-redux'; @@ -23,7 +23,6 @@ import { hardGoTo as navigate } from '../redux/actions'; import { signInLoadingSelector, userSelector, - isSignedInSelector, userTokenSelector } from '../redux/selectors'; import type { User } from '../redux/prop-types'; @@ -44,7 +43,6 @@ const { apiLocation } = envData; // TODO: update types for actions type ShowSettingsProps = { createFlashMessage: typeof createFlashMessage; - isSignedIn: boolean; navigate: (location: string) => void; showLoading: boolean; toggleSoundMode: (sound: boolean) => void; @@ -61,17 +59,10 @@ type ShowSettingsProps = { const mapStateToProps = createSelector( signInLoadingSelector, userSelector, - isSignedInSelector, userTokenSelector, - ( - showLoading: boolean, - user: User | null, - isSignedIn, - userToken: string | null - ) => ({ + (showLoading: boolean, user: User | null, userToken: string | null) => ({ showLoading, user, - isSignedIn, userToken }) ); @@ -94,7 +85,6 @@ export function ShowSettings(props: ShowSettingsProps): JSX.Element { const { t } = useTranslation(); const { createFlashMessage, - isSignedIn, toggleSoundMode, toggleKeyboardShortcuts, resetEditorLayout, @@ -107,8 +97,6 @@ export function ShowSettings(props: ShowSettingsProps): JSX.Element { userToken } = props; - const isSignedInRef = useRef(isSignedIn); - const handleHashChange = () => { const id = window.location.hash.replace('#', ''); if (id) { @@ -127,12 +115,11 @@ export function ShowSettings(props: ShowSettingsProps): JSX.Element { return () => window.removeEventListener('hashchange', handleHashChange); }, []); - if (showLoading || !user) { - return ; - } + useEffect(() => { + if (!user) navigate(`${apiLocation}/signin`); + }, [user, navigate]); - if (!isSignedInRef.current) { - navigate(`${apiLocation}/signin`); + if (showLoading || !user) { return ; } diff --git a/client/src/redux/create-store.ts b/client/src/redux/create-store.ts index fa087105b48..acea425c1e5 100644 --- a/client/src/redux/create-store.ts +++ b/client/src/redux/create-store.ts @@ -47,7 +47,6 @@ export const createStore = (preloadedState = {}) => { preloadedState }); sagaMiddleware.run(rootSaga); - // eslint-disable-next-line @typescript-eslint/no-unsafe-argument epicMiddleware.run(rootEpic); if (module.hot) { diff --git a/client/src/redux/hard-go-to-epic.js b/client/src/redux/hard-go-to-epic.js index 53a4cb302a5..729a4eded56 100644 --- a/client/src/redux/hard-go-to-epic.js +++ b/client/src/redux/hard-go-to-epic.js @@ -3,11 +3,12 @@ import { tap, ignoreElements } from 'rxjs/operators'; import { actionTypes } from './action-types'; -export default function hardGoToEpic(action$, _, { location }) { +// The third argument contains dependencies, see createEpicMiddleware +export default function hardGoToEpic(action$, _, { window }) { return action$.pipe( ofType(actionTypes.hardGoTo), tap(({ payload }) => { - location.href = payload; + window.location.href = payload; }), ignoreElements() ); diff --git a/client/src/redux/index.js b/client/src/redux/index.js index edf7a47c84e..428b183ec40 100644 --- a/client/src/redux/index.js +++ b/client/src/redux/index.js @@ -49,7 +49,8 @@ export const defaultDonationFormState = { } }; -const initialState = { +// exported for testing purposes. +export const initialState = { isRandomCompletionThreshold: false, donatableSectionRecentlyCompleted: null, currentChallengeId: store.get(CURRENT_CHALLENGE_KEY), diff --git a/client/src/redux/root-epic.ts b/client/src/redux/root-epic.ts index 34c26a2a377..81a0a6d7712 100644 --- a/client/src/redux/root-epic.ts +++ b/client/src/redux/root-epic.ts @@ -3,7 +3,6 @@ import { combineEpics } from 'redux-observable'; import { epics as challengeEpics } from '../templates/Challenges/redux'; import { epics as appEpics } from '.'; -// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const rootEpic = combineEpics(...appEpics, ...challengeEpics); export default rootEpic; diff --git a/client/src/redux/settings/danger-zone-saga.js b/client/src/redux/settings/danger-zone-saga.js index 40af64d0c9f..f302b1b6f2d 100644 --- a/client/src/redux/settings/danger-zone-saga.js +++ b/client/src/redux/settings/danger-zone-saga.js @@ -1,4 +1,5 @@ -import { navigate } from 'gatsby'; +import { withPrefix } from 'gatsby'; +import { navigate } from '@gatsbyjs/reach-router'; import { call, put, take, takeEvery } from 'redux-saga/effects'; import { createFlashMessage } from '../../components/Flash/redux'; @@ -17,9 +18,13 @@ function* deleteAccountSaga() { message: FlashMessages.AccountDeleted }) ); + // navigate before signing out, since /settings will attempt to sign users + // back in. Using reach-router's navigate because gatsby's resolves after + // the call. This would allow resetUserData to take place while the user is + // still on /settings. + yield call(navigate, withPrefix('/learn')); // remove current user information from application state yield put(resetUserData()); - yield call(navigate, '/learn'); } catch (e) { yield put(deleteAccountError(e)); }