ronantakizawa/codereview-bench huggingface dataset:
Repository: freeCodeCamp/freeCodeCamp
File: e2e/search-bar-optimized.spec.ts (TypeScript)

Code before:
  });

  test('should clear the search input when the user clicks the clear button', async ({
    page,
    isMobile
  }) => {
    const searchInput = await getSearchInput({ page, isMobile });
    await expect(searchInput).toBeVisible();

    await searchInput.fill('test');
    await page
      .getByRole('button', { name: translations.icons['input-reset'] })
      .click();

    await expect(searchInput).toHaveValue('');
  });

  test('should not exist when the user navigates to a different page from the root', async ({
    page
  }) => {
    await page.getByTestId('curriculum-map-button').nth(0).click();

    // Any of the searchbar elements wont be visible for a certain amount of time, thus the timeout.
    await page.waitForTimeout(500);

    await expect(page.getByTestId('search-optimized')).not.toBeVisible();
  });

  test('should exist when the user navigates back to the root page', async ({
    page,
    isMobile
  }) => {
    await page.getByTestId('curriculum-map-button').nth(0).click();

    // If we do not wait here the history is not yet pushed in to
    // the browser context and the back navigation will default to about: blank
    await page.waitForTimeout(500);

    await page.goBack();
    await getSearchInput({ page, isMobile });
    await expect(page.getByTestId('search-optimized')).toBeVisible();
  });
});


Review comment (performance):
Rather than looking for the absence of something and waiting arbitrarily, how about waiting for the non-optimized search bar to appear? i.e. add `search-unoptimized` to the other search component and look for that.

Code after:
  });

  test('should clear the search input when the user clicks the clear button', async ({
    page,
    isMobile
  }) => {
    const searchInput = await getSearchInput({ page, isMobile });
    await expect(searchInput).toBeVisible();

    await searchInput.fill('test');
    await page
      .getByRole('button', { name: translations.icons['input-reset'] })
      .click();

    await expect(searchInput).toHaveValue('');
  });

  test('The optimized searchbar component should not render when not on the landing page', async ({
    page,
    isMobile
  }) => {
    // This means that the default search bar should be rendered ^.
    await page.getByTestId('curriculum-map-button').nth(0).click();

    if (isMobile) {
      const menuButton = page.getByTestId('header-menu-button');
      await expect(menuButton).toBeVisible();
      await menuButton.click();
    }

    await expect(page.getByTestId('header-search')).toBeVisible();
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/Header/components/universal-nav.tsx (TypeScript)

Code before:

type UniversalNavProps = Omit<
  NavLinksProps,
  'toggleNightMode' | 'openSignoutModal'
> & {
  fetchState: { pending: boolean };
  searchBarRef?: React.RefObject<HTMLDivElement>;
  pathname: string;
};
const UniversalNav = ({
  displayMenu,
  showMenu,
  hideMenu,
  menuButtonRef,
  searchBarRef,
  user,
  fetchState,
  pathname
}: UniversalNavProps): JSX.Element => {
  const { pending } = fetchState;
  const { t } = useTranslation();
  const isSearchExposedWidth = useMediaQuery({
    query: `(min-width: ${SEARCH_EXPOSED_WIDTH}px)`
  });

  const search =
    typeof window !== `undefined` && isLanding(pathname) ? (
      <SearchBarOptimized innerRef={searchBarRef} />
    ) : (
      <SearchBar innerRef={searchBarRef} />
    );
  return (
    <nav
      aria-label={t('aria.primary-nav')}
      className='universal-nav'
      id='universal-nav'
      data-playwright-test-label='header-universal-nav'
    >
      {isSearchExposedWidth && (
        <div className='universal-nav-left'>{search}</div>
      )}
      <Link
        className='universal-nav-logo'
        id='universal-nav-logo'
        to='/learn'
        data-playwright-test-label='header-universal-nav-logo'
      >
        <NavLogo />
      </Link>
      <div className='universal-nav-right main-nav'>
        {pending ? (

Review comment (suggestion):
```suggestion
  const search = isLanding(pathname) ? (
```
since we're not using window, we don't have to check it exists.

Code after:

type UniversalNavProps = Omit<
  NavLinksProps,
  'toggleNightMode' | 'openSignoutModal'
> & {
  fetchState: { pending: boolean };
  searchBarRef?: React.RefObject<HTMLDivElement>;
  pathname: string;
};
const UniversalNav = ({
  displayMenu,
  showMenu,
  hideMenu,
  menuButtonRef,
  searchBarRef,
  user,
  fetchState,
  pathname
}: UniversalNavProps): JSX.Element => {
  const { pending } = fetchState;
  const { t } = useTranslation();
  const isSearchExposedWidth = useMediaQuery({
    query: `(min-width: ${SEARCH_EXPOSED_WIDTH}px)`
  });

  const search = isLanding(pathname) ? (
    <SearchBarOptimized innerRef={searchBarRef} />
  ) : (
    <SearchBar innerRef={searchBarRef} />
  );
  return (
    <nav
      aria-label={t('aria.primary-nav')}
      className='universal-nav'
      id='universal-nav'
      data-playwright-test-label='header-universal-nav'
    >
      {isSearchExposedWidth && (
        <div className='universal-nav-left'>{search}</div>
      )}
      <Link
        className='universal-nav-logo'
        id='universal-nav-logo'
        to='/learn'
        data-playwright-test-label='header-universal-nav-logo'
      >
        <NavLogo />
      </Link>
      <div className='universal-nav-right main-nav'>
        {pending ? (
          <div className='nav-skeleton'>
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/Header/components/universal-nav.tsx (TypeScript)

Code before:
  fetchState: { pending: boolean };
  searchBarRef?: React.RefObject<HTMLDivElement>;
  pathname: string;
};
const UniversalNav = ({
  displayMenu,
  showMenu,
  hideMenu,
  menuButtonRef,
  searchBarRef,
  user,
  fetchState,
  pathname
}: UniversalNavProps): JSX.Element => {
  const { pending } = fetchState;
  const { t } = useTranslation();
  const isSearchExposedWidth = useMediaQuery({
    query: `(min-width: ${SEARCH_EXPOSED_WIDTH}px)`
  });

  const search =
    typeof window !== `undefined` && isLanding(pathname) ? (
      <SearchBarOptimized innerRef={searchBarRef} />
    ) : (
      <SearchBar innerRef={searchBarRef} />
    );
  return (
    <nav
      aria-label={t('aria.primary-nav')}
      className='universal-nav'
      id='universal-nav'
      data-playwright-test-label='header-universal-nav'
    >
      {isSearchExposedWidth && (
        <div className='universal-nav-left'>{search}</div>
      )}
      <Link
        className='universal-nav-logo'
        id='universal-nav-logo'
        to='/learn'
        data-playwright-test-label='header-universal-nav-logo'
      >
        <NavLogo />
      </Link>
      <div className='universal-nav-right main-nav'>
        {pending ? (
          <div className='nav-skeleton'>
            <SkeletonSprite />
          </div>
        ) : (
          <>

Review comment (performance):
```suggestion
    <SearchBarOptimized innerRef={searchBarRef} />
  ) : (
    <SearchBar innerRef={searchBarRef} />
  );
```
To appease prettier

Code after:
  fetchState: { pending: boolean };
  searchBarRef?: React.RefObject<HTMLDivElement>;
  pathname: string;
};
const UniversalNav = ({
  displayMenu,
  showMenu,
  hideMenu,
  menuButtonRef,
  searchBarRef,
  user,
  fetchState,
  pathname
}: UniversalNavProps): JSX.Element => {
  const { pending } = fetchState;
  const { t } = useTranslation();
  const isSearchExposedWidth = useMediaQuery({
    query: `(min-width: ${SEARCH_EXPOSED_WIDTH}px)`
  });

  const search = isLanding(pathname) ? (
    <SearchBarOptimized innerRef={searchBarRef} />
  ) : (
    <SearchBar innerRef={searchBarRef} />
  );
  return (
    <nav
      aria-label={t('aria.primary-nav')}
      className='universal-nav'
      id='universal-nav'
      data-playwright-test-label='header-universal-nav'
    >
      {isSearchExposedWidth && (
        <div className='universal-nav-left'>{search}</div>
      )}
      <Link
        className='universal-nav-logo'
        id='universal-nav-logo'
        to='/learn'
        data-playwright-test-label='header-universal-nav-logo'
      >
        <NavLogo />
      </Link>
      <div className='universal-nav-right main-nav'>
        {pending ? (
          <div className='nav-skeleton'>
            <SkeletonSprite />
          </div>
        ) : (
          <>
            <LanguageList />
Repository: freeCodeCamp/freeCodeCamp
File: api/src/routes/protected/challenge.ts (TypeScript)

Code before:
        select: {
          id: true,
          quizAttempts: true
        }
      });

      const existingAttemptIdx = user.quizAttempts.findIndex(
        attempt =>
          attempt.challengeId === challengeId && attempt.quizId === quizId
      );

      const newAttempt = {
        challengeId,
        quizId,
        timestamp: Date.now()
      };

      const newQuizAttempts = [...user.quizAttempts];

      // If there is an existing attempt, replace it with the new attempt (essentially only timestamp is changed).
      // Otherwise, add the new attempt to the list.
      if (existingAttemptIdx !== -1) {
        newQuizAttempts.splice(existingAttemptIdx, 1, newAttempt);
      } else {
        newQuizAttempts.push(newAttempt);
      }

      await fastify.prisma.user.update({
        where: { id: user.id },
        data: {
          quizAttempts: newQuizAttempts
        }
      });

      return {
        attemptDate: newAttempt.timestamp
      };
    }
  );

  done();
};


Review comment (bug):
What I have in mind is:
- The client calls this API when the camper clicks the submit quiz button, regardless of the quiz result.
- On the backend, we store a single attempt for each quiz, and the attempt is the latest one.

Using the `completedChallenges` array, we know whether or not the camper has passed the quiz. And the attempt information here allows us to:
- Enforce a cool-down period if the camper has failed the quiz.
- Use a different quiz question set if the camper has already passed the quiz and wants to re-do it (we only have a single question set at the moment, but this could be a future feature).


Code after:
        select: {
          id: true,
          quizAttempts: true
        }
      });

      const existingAttempt = user.quizAttempts.find(matches({ challengeId }));

      const newAttempt = {
        challengeId,
        quizId,
        timestamp: Date.now()
      };

      await fastify.prisma.user.update({
        where: { id: user.id },
        data: {
          quizAttempts: existingAttempt
            ? {
                updateMany: { where: { challengeId }, data: newAttempt }
              }
            : { push: newAttempt }
        }
      });

      return {};
    }
  );

  done();
};
Repository: freeCodeCamp/freeCodeCamp
File: api/src/routes/protected/challenge.ts (TypeScript)

Code before:
          void reply.code(400);
          void reply.send({
            type: 'error',
            message:
              'That does not appear to be a valid quiz attempt submission.'
          });
        } else {
          fastify.errorHandler(error, request, reply);
        }
      }
    },
    async req => {
      const { challengeId, quizId } = req.body;

      const user = await fastify.prisma.user.findUniqueOrThrow({
        where: { id: req.user?.id },
        select: {
          id: true,
          quizAttempts: true
        }
      });

      const existingAttemptIdx = user.quizAttempts.findIndex(
        attempt =>
          attempt.challengeId === challengeId && attempt.quizId === quizId
      );

      const newAttempt = {
        challengeId,
        quizId,
        timestamp: Date.now()
      };

      const newQuizAttempts = [...user.quizAttempts];

      // If there is an existing attempt, replace it with the new attempt (essentially only timestamp is changed).
      // Otherwise, add the new attempt to the list.
      if (existingAttemptIdx !== -1) {
        newQuizAttempts.splice(existingAttemptIdx, 1, newAttempt);
      } else {
        newQuizAttempts.push(newAttempt);
      }

      await fastify.prisma.user.update({
        where: { id: user.id },
        data: {
          quizAttempts: newQuizAttempts
        }
      });

      return {

Review comment (suggestion):
```suggestion
      const existingAttempt = user.quizAttempts.find(
        matches({ quizId, challengeId })
      );
```
lodash's matches cleans this up a little, imo. 

The change from findIndex to find is mostly cosmetic. If we use the mongo array manipulation methods (see below!) we only care if we've found something, not which index it is.

Code after:
          void reply.code(400);
          void reply.send({
            type: 'error',
            message:
              'That does not appear to be a valid quiz attempt submission.'
          });
        } else {
          fastify.errorHandler(error, request, reply);
        }
      }
    },
    async req => {
      const { challengeId, quizId } = req.body;

      const user = await fastify.prisma.user.findUniqueOrThrow({
        where: { id: req.user?.id },
        select: {
          id: true,
          quizAttempts: true
        }
      });

      const existingAttempt = user.quizAttempts.find(matches({ challengeId }));

      const newAttempt = {
        challengeId,
        quizId,
        timestamp: Date.now()
      };

      await fastify.prisma.user.update({
        where: { id: user.id },
        data: {
          quizAttempts: existingAttempt
            ? {
                updateMany: { where: { challengeId }, data: newAttempt }
              }
            : { push: newAttempt }
        }
      });

      return {};
    }
  );

  done();
};
Repository: freeCodeCamp/freeCodeCamp
File: client/src/utils/math-jax.ts (TypeScript)

Code before:
import { SuperBlocks } from '../../../shared/config/curriculum';
import { scriptLoader } from './script-loaders';

export const mathJaxSrc =
  'https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.4/MathJax.js?config=TeX-AMS_HTML';

export const isMathJaxAllowed = (pathname: string) =>
  initiateMathJaxOnSuperBlock.some(superBlock => pathname.includes(superBlock));

const initiateMathJaxOnSuperBlock = [
  SuperBlocks.JsAlgoDataStructNew,
  SuperBlocks.JsAlgoDataStruct,
  SuperBlocks.ProjectEuler,
  SuperBlocks.RosettaCode,
  SuperBlocks.SciCompPy
];

const configure = () => {
  if (!global.MathJax) return;
  const MathJax = global.MathJax;
  MathJax.Hub.Config({
    tex2jax: {
      inlineMath: [
        ['$', '$'],
        ['\\(', '\\)']
      ],
      processEscapes: true,
      processClass: 'mathjax-support'
    }
  });
  MathJax.Hub.Queue([
    'Typeset',
    MathJax.Hub,
    document.querySelector('.mathjax-support')
  ]);

Review comment (suggestion):
```suggestion
  superBlocksWithMathJax.some(superBlock => pathname.includes(superBlock));

const superBlocksWithMathJax = [
```

Just a naming nit. `initiateMathJax` sounds like a function that initialises Mathjax, not an array of anything.

Code after:
import { SuperBlocks } from '../../../shared/config/curriculum';
import { scriptLoader } from './script-loaders';

export const mathJaxSrc =
  'https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.4/MathJax.js?config=TeX-AMS_HTML';

export const isMathJaxAllowed = (pathname: string) =>
  superBlocksWithMathJax.some(superBlock => pathname.includes(superBlock));

const superBlocksWithMathJax = [
  SuperBlocks.JsAlgoDataStructNew,
  SuperBlocks.JsAlgoDataStruct,
  SuperBlocks.ProjectEuler,
  SuperBlocks.RosettaCode,
  SuperBlocks.SciCompPy
];

const configure = () => {
  if (!global.MathJax) return;
  const MathJax = global.MathJax;
  MathJax.Hub.Config({
    tex2jax: {
      inlineMath: [
        ['$', '$'],
        ['\\(', '\\)']
      ],
      processEscapes: true,
      processClass: 'mathjax-support'
    }
  });
  MathJax.Hub.Queue([
    'Typeset',
    MathJax.Hub,
    document.querySelector('.mathjax-support')
  ]);
Repository: freeCodeCamp/freeCodeCamp
File: client/src/utils/algolia-locale-setup.ts (TypeScript)

Code before:
  clientLocale: keyof typeof algoliaIndices;
};

const algoliaIndices = {
  english: {
    name: 'news',
    searchPage: 'https://www.freecodecamp.org/news/search/'
  },
  espanol: {
    name: 'news-es',
    searchPage: 'https://www.freecodecamp.org/espanol/news/search/'
  },
  chinese: {
    name: 'news-zh',
    searchPage: 'https://www.freecodecamp.org/chinese/news/search/'
  },
  'chinese-traditional': {
    name: 'news-zh',
    searchPage: 'https://www.freecodecamp.org/chinese/news/search/'
  },
  italian: {
    name: 'news-it',
    searchPage: 'https://www.freecodecamp.org/italian/news/search/'
  },
  portuguese: {
    name: 'news-pt-br',
    searchPage: 'https://www.freecodecamp.org/portuguese/news/search/'
  },
  ukrainian: {
    name: 'news-uk',
    searchPage: 'https://www.freecodecamp.org/ukrainian/news/search/'
  },
  japanese: {
    name: 'news-ja',
    searchPage: 'https://www.freecodecamp.org/japanese/news/search/'
  },
  korean: {
    name: 'news-ko',
    searchPage: 'https://www.freecodecamp.org/korean/news/search/'
  },
  // Note: We don't build News for the locales below, so show English
  // hits and use the English search page
  german: {
    name: 'news',
    searchPage: 'https://www.freecodecamp.org/news/search/'
  },
  swahili: {
    name: 'news',
    searchPage: 'https://www.freecodecamp.org/swahili/news/search/'
  }
};

Review comment (suggestion):
I only vaguely remember how to configure the search, but do these values need to match with the `algoliaIndices` values in the /news repo here? https://github.com/freeCodeCamp/news/blob/main/config/index.js

Code after:
  clientLocale: keyof typeof algoliaIndices;
};

const algoliaIndices = {
  english: {
    name: 'news',
    searchPage: 'https://www.freecodecamp.org/news/search/'
  },
  espanol: {
    name: 'news-es',
    searchPage: 'https://www.freecodecamp.org/espanol/news/search/'
  },
  chinese: {
    name: 'news-zh',
    searchPage: 'https://www.freecodecamp.org/chinese/news/search/'
  },
  'chinese-traditional': {
    name: 'news-zh',
    searchPage: 'https://www.freecodecamp.org/chinese/news/search/'
  },
  italian: {
    name: 'news-it',
    searchPage: 'https://www.freecodecamp.org/italian/news/search/'
  },
  portuguese: {
    name: 'news-pt-br',
    searchPage: 'https://www.freecodecamp.org/portuguese/news/search/'
  },
  ukrainian: {
    name: 'news-uk',
    searchPage: 'https://www.freecodecamp.org/ukrainian/news/search/'
  },
  japanese: {
    name: 'news-ja',
    searchPage: 'https://www.freecodecamp.org/japanese/news/search/'
  },
  korean: {
    name: 'news-ko',
    searchPage: 'https://www.freecodecamp.org/korean/news/search/'
  },
  // Note: We don't build News for the locales below, so show English
  // hits and use the English search page
  german: {
    name: 'news',
    searchPage: 'https://www.freecodecamp.org/news/search/'
  },
  swahili: {
    name: 'news',
    searchPage: 'https://www.freecodecamp.org/news/search/'
  }
};
Repository: freeCodeCamp/freeCodeCamp
File: client/src/utils/algolia-locale-setup.ts (TypeScript)

Code before:
  },
  portuguese: {
    name: 'news-pt-br',
    searchPage: 'https://www.freecodecamp.org/portuguese/news/search/'
  },
  ukrainian: {
    name: 'news-uk',
    searchPage: 'https://www.freecodecamp.org/ukrainian/news/search/'
  },
  japanese: {
    name: 'news-ja',
    searchPage: 'https://www.freecodecamp.org/japanese/news/search/'
  },
  korean: {
    name: 'news-ko',
    searchPage: 'https://www.freecodecamp.org/korean/news/search/'
  },
  // Note: We don't build News for the locales below, so show English
  // hits and use the English search page
  german: {
    name: 'news',
    searchPage: 'https://www.freecodecamp.org/news/search/'
  },
  swahili: {
    name: 'news',
    searchPage: 'https://www.freecodecamp.org/swahili/news/search/'
  }
};

export const newsIndex = algoliaIndices[clientLocale].name;
export const searchPageUrl = algoliaIndices[clientLocale].searchPage;


Review comment (suggestion):
```suggestion
    searchPage: 'https://www.freecodecamp.org/news/search/'
```
There's no swahili news at the moment.

Code after:
  },
  portuguese: {
    name: 'news-pt-br',
    searchPage: 'https://www.freecodecamp.org/portuguese/news/search/'
  },
  ukrainian: {
    name: 'news-uk',
    searchPage: 'https://www.freecodecamp.org/ukrainian/news/search/'
  },
  japanese: {
    name: 'news-ja',
    searchPage: 'https://www.freecodecamp.org/japanese/news/search/'
  },
  korean: {
    name: 'news-ko',
    searchPage: 'https://www.freecodecamp.org/korean/news/search/'
  },
  // Note: We don't build News for the locales below, so show English
  // hits and use the English search page
  german: {
    name: 'news',
    searchPage: 'https://www.freecodecamp.org/news/search/'
  },
  swahili: {
    name: 'news',
    searchPage: 'https://www.freecodecamp.org/news/search/'
  }
};

export const newsIndex = algoliaIndices[clientLocale].name;
export const searchPageUrl = algoliaIndices[clientLocale].searchPage;
Repository: freeCodeCamp/freeCodeCamp
File: api/src/exam-environment/routes/exam-environment.ts (TypeScript)

Code before:
  reply: FastifyReply
) {
  const { 'exam-environment-authorization-token': encodedToken } = req.headers;

  let payload: JwtPayload;
  try {
    payload = jwt.verify(encodedToken, JWT_SECRET) as JwtPayload;
  } catch (e) {
    // Server refuses to brew (verify) coffee (jwts) with a teapot (random strings)
    void reply.code(418);
    return reply.send(
      ERRORS.FCC_EINVAL_EXAM_ENVIRONMENT_AUTHORIZATION_TOKEN(JSON.stringify(e))
    );
  }

  const tokenId = isObjectID(payload.examEnvironmentAuthorizationToken)
    ? payload.examEnvironmentAuthorizationToken
    : null;

  const token = tokenId
    ? await this.prisma.examEnvironmentAuthorizationToken.findUnique({
        where: {
          id: tokenId
        }
      })
    : null;

  if (!token) {
    // Endpoint is valid, but resource does not exists
    void reply.code(404);
    return reply.send(
      ERRORS.FCC_EINVAL_EXAM_ENVIRONMENT_AUTHORIZATION_TOKEN(
        'Token does not appear to exist'
      )
    );
  } else {
    void reply.code(200);
    return reply.send({
      expireAt: token.expireAt
    });
  }
}

/**
 * Generates an exam for the user.
 *
 * Requires token to be validated and TODO: live longer than the exam attempt.
 */
async function postExamGeneratedExamHandler(
  this: FastifyInstance,
  req: UpdateReqType<typeof schemas.examEnvironmentPostExamGeneratedExam>,

Review comment (security):
```suggestion
  const token = await this.prisma.examEnvironmentAuthorizationToken.findUnique({
    where: {
      id: payload.examEnvironmentAuthorizationToken
    }
  });
```

Code after:
  reply: FastifyReply
) {
  const { 'exam-environment-authorization-token': encodedToken } = req.headers;

  let payload: JwtPayload;
  try {
    payload = jwt.verify(encodedToken, JWT_SECRET) as JwtPayload;
  } catch (e) {
    // Server refuses to brew (verify) coffee (jwts) with a teapot (random strings)
    void reply.code(418);
    return reply.send(
      ERRORS.FCC_EINVAL_EXAM_ENVIRONMENT_AUTHORIZATION_TOKEN(JSON.stringify(e))
    );
  }

  if (!isObjectID(payload.examEnvironmentAuthorizationToken)) {
    void reply.code(418);
    return reply.send(
      ERRORS.FCC_EINVAL_EXAM_ENVIRONMENT_AUTHORIZATION_TOKEN(
        'Token is not valid'
      )
    );
  }

  const token = await this.prisma.examEnvironmentAuthorizationToken.findUnique({
    where: {
      id: payload.examEnvironmentAuthorizationToken
    }
  });

  if (!token) {
    // Endpoint is valid, but resource does not exists
    void reply.code(404);
    return reply.send(
      ERRORS.FCC_EINVAL_EXAM_ENVIRONMENT_AUTHORIZATION_TOKEN(
        'Token does not appear to exist'
      )
    );
  } else {
    void reply.code(200);
    return reply.send({
      expireAt: token.expireAt
    });
  }
}

/**
 * Generates an exam for the user.
 *
 * Requires token to be validated and TODO: live longer than the exam attempt.
 */