fix: redirect to api /signin from client /settings (#64398)

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<string, string>;
+vi.mock('../utils/get-words');
+
+const { apiLocation } = envData;
 
 describe('<ShowSettings />', () => {
-  it('renders to the DOM when user is logged in', () => {
-    const shallow = new ShallowRenderer();
-    shallow.render(<ShowSettings {...loggedInProps} />);
-    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(
+      <Provider store={store}>
+        <ShowSettings />
+      </Provider>
     );
+    expect(spy).toHaveBeenCalledTimes(0);
   });
 
   it('redirects to sign in page when user is not logged in', () => {
-    const shallow = new ShallowRenderer();
-    shallow.render(<ShowSettings {...loggedOutProps} />);
-    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(
+      <Provider store={store}>
+        <ShowSettings />
+      </Provider>
+    );
+    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;

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 <Loader fullScreen={true} />;
-  }
+  useEffect(() => {
+    if (!user) navigate(`${apiLocation}/signin`);
+  }, [user, navigate]);
 
-  if (!isSignedInRef.current) {
-    navigate(`${apiLocation}/signin`);
+  if (showLoading || !user) {
     return <Loader fullScreen={true} />;
   }
 

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) {

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()
   );

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),

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;

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));
   }