refactor: replace isChallenge (#50033)

* refactor: replace isChallenge

Determining if a path is a challenge by the number of path segments is
brittle and we ended up writing bizarre things like
isChallenge(nextChallengePath).

This should be a little more robust. i.e. if we need to know if a page
is a challenge, we can check the challengeMeta

* test: update tests with new logic

client/src/redux/prop-types.ts

@@ -292,8 +292,8 @@ export type ChallengeMeta = {
   id: string;
   introPath: string;
   isFirstStep: boolean;
-  nextChallengePath: string;
-  prevChallengePath: string;
+  nextChallengePath: string | null;
+  prevChallengePath: string | null;
   removeComments: boolean;
   superBlock: SuperBlocks;
   title?: string;

client/src/templates/Challenges/components/hotkeys.tsx

@@ -4,8 +4,12 @@ import { HotKeys, GlobalHotKeys } from 'react-hotkeys';
 import { connect } from 'react-redux';
 import { createSelector } from 'reselect';
 import { editor } from 'monaco-editor';
-import { ChallengeFiles, Test, User } from '../../../redux/prop-types';
-import { isChallenge } from '../../../utils/path-parsers';
+import type {
+  ChallengeFiles,
+  Test,
+  User,
+  ChallengeMeta
+} from '../../../redux/prop-types';
 
 import { userSelector } from '../../../redux/selectors';
 import {
@@ -57,7 +61,8 @@ const keyMap = {
   showShortcuts: 'shift+/'
 };
 
-interface HotkeysProps {
+interface HotkeysProps
+  extends Pick<ChallengeMeta, 'nextChallengePath' | 'prevChallengePath'> {
   canFocusEditor: boolean;
   challengeFiles: ChallengeFiles;
   challengeType?: number;
@@ -67,8 +72,6 @@ interface HotkeysProps {
   submitChallenge: () => void;
   innerRef: MutableRefObject<HTMLElement | undefined>;
   instructionsPanelRef?: React.RefObject<HTMLElement>;
-  nextChallengePath: string;
-  prevChallengePath: string;
   setEditorFocusability: (arg0: boolean) => void;
   setIsAdvancing: (arg0: boolean) => void;
   tests: Test[];
@@ -137,14 +140,22 @@ function Hotkeys({
           navigationMode: () => setEditorFocusability(false),
           navigatePrev: () => {
             if (!canFocusEditor) {
-              if (isChallenge(prevChallengePath)) setIsAdvancing(true);
-              void navigate(prevChallengePath);
+              if (prevChallengePath) {
+                setIsAdvancing(true);
+                void navigate(prevChallengePath);
+              } else {
+                void navigate('/learn');
+              }
             }
           },
           navigateNext: () => {
             if (!canFocusEditor) {
-              if (isChallenge(nextChallengePath)) setIsAdvancing(true);
-              void navigate(nextChallengePath);
+              if (nextChallengePath) {
+                setIsAdvancing(true);
+                void navigate(nextChallengePath);
+              } else {
+                void navigate('/learn');
+              }
             }
           },
           showShortcuts: (e: React.KeyboardEvent) => {

client/src/templates/Challenges/redux/completion-epic.js

@@ -10,7 +10,6 @@ import {
   tap,
   mergeMap
 } from 'rxjs/operators';
-import { isChallenge } from '../../../utils/path-parsers';
 import { challengeTypes, submitTypes } from '../../../../utils/challenge-types';
 import { actionTypes as submitActionTypes } from '../../../redux/action-types';
 import {
@@ -184,16 +183,19 @@ export default function completionEpic(action$, state$) {
         submitter = submitters[submitTypes[challengeType]];
       }
 
-      const pathToNavigateTo = () => {
-        return findPathToNavigateTo(nextChallengePath, superBlock);
-      };
+      const isNextChallengeInSameSuperBlock =
+        nextChallengePath.includes(superBlock);
+
+      const pathToNavigateTo = isNextChallengeInSameSuperBlock
+        ? nextChallengePath
+        : `/learn/${superBlock}/#${superBlock}-projects`;
 
       const canAllowDonationRequest = (state, action) =>
         isBlockNewlyCompletedSelector(state) &&
         action.type === submitActionTypes.submitComplete;
 
       return submitter(type, state).pipe(
-        concat(of(setIsAdvancing(isChallenge(pathToNavigateTo())))),
+        concat(of(setIsAdvancing(isNextChallengeInSameSuperBlock))),
         mergeMap(x =>
           canAllowDonationRequest(state, x)
             ? of(x, allowBlockDonationRequests({ superBlock, block }))
@@ -201,7 +203,7 @@ export default function completionEpic(action$, state$) {
         ),
         tap(res => {
           if (res.type !== submitActionTypes.updateFailed) {
-            navigate(pathToNavigateTo());
+            navigate(pathToNavigateTo);
           }
         }),
         concat(of(closeModal('completion')))
@@ -209,11 +211,3 @@ export default function completionEpic(action$, state$) {
     })
   );
 }
-
-function findPathToNavigateTo(nextChallengePath, superBlock) {
-  if (nextChallengePath.includes(superBlock)) {
-    return nextChallengePath;
-  } else {
-    return `/learn/${superBlock}/#${superBlock}-projects`;
-  }
-}

client/src/utils/path-parsers.test.ts

@@ -1,4 +1,4 @@
-import { isChallenge, isLanding } from './path-parsers';
+import { isLanding } from './path-parsers';
 
 const pathnames = {
   english: {
@@ -56,33 +56,3 @@ describe('isLanding', () => {
     expect(isLanding(pathnames.espanolWithYear.challenge)).toBe(false);
   });
 });
-
-describe('isChallenge', () => {
-  it('returns a boolean', () => {
-    expect(typeof isChallenge('/')).toBe('boolean');
-  });
-  it('returns false for Espanol landing pathname', () => {
-    expect(isChallenge(pathnames.espanol.landing)).toBe(false);
-  });
-  it('returns false for Espanol super block pathname', () => {
-    expect(isChallenge(pathnames.espanol.superBlock)).toBe(false);
-  });
-  it('returns true for Espanol challenge pathname', () => {
-    expect(isChallenge(pathnames.espanol.challenge)).toBe(true);
-  });
-  it('returns false for English landing pathname', () => {
-    expect(isChallenge(pathnames.english.landing)).toBe(false);
-  });
-  it('returns false for English super block pathname', () => {
-    expect(isChallenge(pathnames.english.superBlock)).toBe(false);
-  });
-  it('returns true for English challenge pathname', () => {
-    expect(isChallenge(pathnames.english.challenge)).toBe(true);
-  });
-  it('returns true for English with year challenge pathname', () => {
-    expect(isChallenge(pathnames.englishWithYear.challenge)).toBe(true);
-  });
-  it('returns true for Espanol with year challenge pathname', () => {
-    expect(isChallenge(pathnames.espanolWithYear.challenge)).toBe(true);
-  });
-});

client/src/utils/path-parsers.ts

@@ -4,20 +4,6 @@ import { i18nConstants } from '../../../config/constants';
 const splitPath = (pathname: string): string[] =>
   pathname.split('/').filter(x => x);
 
-export const isChallenge = (pathname: string): boolean => {
-  const pathArray = splitPath(pathname);
-  return (
-    // learn/<superBlock>/<block>/<challenge>
-    (pathArray.length === 4 && pathArray[0] === 'learn') ||
-    // learn/<year>/<superBlock>/<block>/<challenge>
-    (pathArray.length === 5 && pathArray[0] === 'learn') ||
-    // <i18n>/learn/<superBlock>/<block>/<challenge>
-    (pathArray.length === 5 && pathArray[1] === 'learn') ||
-    // <i18n>/learn/<year>/<superBlock>/<block>/<challenge>
-    (pathArray.length === 6 && pathArray[1] === 'learn')
-  );
-};
-
 export const isLanding = (pathname: string): boolean => {
   const pathArray = splitPath(pathname);
   const isEnglishLanding = pathArray.length === 0;

client/utils/gatsby/challenge-page-creator.js

@@ -58,12 +58,12 @@ function getIsFirstStep(_node, index, nodeArray) {
 
 function getNextChallengePath(_node, index, nodeArray) {
   const next = nodeArray[index + 1];
-  return next ? next.node.challenge.fields.slug : '/learn';
+  return next ? next.node.challenge.fields.slug : null;
 }
 
 function getPrevChallengePath(_node, index, nodeArray) {
   const prev = nodeArray[index - 1];
-  return prev ? prev.node.challenge.fields.slug : '/learn';
+  return prev ? prev.node.challenge.fields.slug : null;
 }
 
 function getTemplateComponent(challengeType) {

client/utils/gatsby/layout-selector.test.tsx

@@ -19,7 +19,8 @@ interface NameAndProps {
 }
 function getComponentNameAndProps(
   elementType: React.JSXElementConstructor<never>,
-  pathname: string
+  pathname: string,
+  pageContext?: { challengeMeta?: { block?: string; superBlock?: string } }
 ): NameAndProps {
   // eslint-disable-next-line testing-library/render-result-naming-convention
   const shallow = ShallowRenderer.createRenderer();
@@ -28,7 +29,8 @@ function getComponentNameAndProps(
     props: {
       location: {
         pathname
-      }
+      },
+      pageContext
     }
   });
   shallow.render(<Provider store={store}>{LayoutReactComponent}</Provider>);
@@ -44,10 +46,21 @@ function getComponentNameAndProps(
   };
 }
 
-test('Challenge path should have DefaultLayout and no footer', () => {
+const challengePageContext = {
+  challengeMeta: {
+    block: 'Basic HTML and HTML5',
+    superBlock: 'responsive-web-design'
+  }
+};
+
+test('Challenges should have DefaultLayout and no footer', () => {
   const challengePath =
     '/learn/responsive-web-design/basic-html-and-html5/say-hello-to-html-elements';
-  const compnentObj = getComponentNameAndProps(Learn, challengePath);
+  const compnentObj = getComponentNameAndProps(
+    Learn,
+    challengePath,
+    challengePageContext
+  );
   expect(compnentObj.name).toEqual('DefaultLayout');
   expect(compnentObj.props.showFooter).toEqual(false);
 });
@@ -59,15 +72,19 @@ test('SuperBlock path should have DefaultLayout and footer', () => {
   expect(compnentObj.props.showFooter).toEqual(true);
 });
 
-test('i18l challenge path should have DefaultLayout and no footer', () => {
+test('i18n challenge path should have DefaultLayout and no footer', () => {
   const challengePath =
     'espanol/learn/responsive-web-design/basic-html-and-html5/say-hello-to-html-elements/';
-  const compnentObj = getComponentNameAndProps(Learn, challengePath);
+  const compnentObj = getComponentNameAndProps(
+    Learn,
+    challengePath,
+    challengePageContext
+  );
   expect(compnentObj.name).toEqual('DefaultLayout');
   expect(compnentObj.props.showFooter).toEqual(false);
 });
 
-test('i18l superBlock path should have DefaultLayout and footer', () => {
+test('i18n superBlock path should have DefaultLayout and footer', () => {
   const superBlockPath = '/learn/responsive-web-design/';
   const compnentObj = getComponentNameAndProps(Learn, superBlockPath);
   expect(compnentObj.name).toEqual('DefaultLayout');

client/utils/gatsby/layout-selector.tsx

@@ -3,7 +3,6 @@ import React from 'react';
 import CertificationLayout from '../../src/components/layouts/certification';
 import DefaultLayout from '../../src/components/layouts/default';
 import FourOhFourPage from '../../src/pages/404';
-import { isChallenge } from '../../src/utils/path-parsers';
 
 interface LayoutSelectorProps {
   element: JSX.Element;
@@ -20,6 +19,8 @@ export default function layoutSelector({
     location: { pathname }
   } = props;
 
+  const isChallenge = !!props.pageContext?.challengeMeta;
+
   if (element.type === FourOhFourPage) {
     return (
       <DefaultLayout pathname={pathname} showFooter={true}>
@@ -30,7 +31,7 @@ export default function layoutSelector({
     return (
       <CertificationLayout pathname={pathname}>{element}</CertificationLayout>
     );
-  } else if (isChallenge(pathname)) {
+  } else if (isChallenge) {
     return (
       <DefaultLayout
         pathname={pathname}