ronantakizawa/codereview-bench huggingface dataset:
Repository: freeCodeCamp/freeCodeCamp
File: api/src/utils/get-challenges.ts (TypeScript)

Code before:
import { flatten } from 'lodash';

// TODO: keeping curriculum in memory is handy if we want to field requests that
// need to 'query' the curriculum, but if we force the client to handle
// redirectToCurrentChallenge and, instead, only report the current challenge id
// via the user object, then we should *not* store this so it can be garbage
// collected.
import curriculum from '../../../config/curriculum.json';
import { SuperBlocks } from '../../../config/superblocks';
import { type ChallengeNode } from '../../../client/src/redux/prop-types';

type Curriculum = { [keyValue in SuperBlocks]?: CurriculumProps };
type SuperBlockKeys = keyof Curriculum;

// eslint-disable-next-line @typescript-eslint/no-unused-vars
interface Block<T> {
  challenges: ChallengeNode['challenge'][];
}

interface CurriculumProps {
  blocks: Record<string, Block<ChallengeNode['challenge'][]>>;
}

export function getChallenges() {
  const superBlockKeys = Object.keys(SuperBlocks) as SuperBlockKeys[];
  const typedCurriculum: Curriculum = curriculum;

  return superBlockKeys
    .map(key => typedCurriculum[key]?.blocks)
    .reduce((accumulator: ChallengeNode['challenge'][], superBlock) => {
      const blockKeys = Object.keys(superBlock ?? {});
      const challengesForBlock = blockKeys.map(
        key => superBlock?.[key]?.challenges ?? []
      );
      return [...accumulator, ...flatten(challengesForBlock)];
    }, []);
}


Review comment (refactor):
```suggestion
interface Block {
  challenges: {
    id: string;
    tests?: { id?: string }[];
    challengeType: number;
  }[];
}
```
This is probably good enough for now, since we're likely refactoring all of this soon.

Code after:
// TODO: keeping curriculum in memory is handy if we want to field requests that
// need to 'query' the curriculum, but if we force the client to handle
// redirectToCurrentChallenge and, instead, only report the current challenge id
// via the user object, then we should *not* store this so it can be garbage
// collected.
import curriculum from '../../../config/curriculum.json';
import { SuperBlocks } from '../../../config/superblocks';

type Curriculum = { [keyValue in SuperBlocks]?: CurriculumProps };
type SuperBlockKeys = keyof Curriculum;

// eslint-disable-next-line @typescript-eslint/no-unused-vars
interface Block {
  challenges: {
    id: string;
    tests?: { id?: string }[];
    challengeType: number;
  }[];
}

interface CurriculumProps {
  blocks: Record<string, Block>;
}

export function getChallenges() {
  const superBlockKeys = Object.keys(SuperBlocks) as SuperBlockKeys[];
  const typedCurriculum: Curriculum = curriculum as Curriculum;

  return superBlockKeys
    .map(key => typedCurriculum[key]?.blocks)
    .reduce((accumulator: Block['challenges'], superBlock) => {
      const blockKeys = Object.keys(superBlock ?? {});
      const challengesForBlock = blockKeys.map(
        key => superBlock?.[key]?.challenges ?? []
      );
      return [...accumulator, ...challengesForBlock.flat()];
    }, []);
}
Repository: freeCodeCamp/freeCodeCamp
File: api/src/utils/get-challenges.ts (TypeScript)

Code before:
import { flatten } from 'lodash';

// TODO: keeping curriculum in memory is handy if we want to field requests that
// need to 'query' the curriculum, but if we force the client to handle
// redirectToCurrentChallenge and, instead, only report the current challenge id
// via the user object, then we should *not* store this so it can be garbage
// collected.
import curriculum from '../../../config/curriculum.json';
import { SuperBlocks } from '../../../config/superblocks';
import { type ChallengeNode } from '../../../client/src/redux/prop-types';

type Curriculum = { [keyValue in SuperBlocks]?: CurriculumProps };
type SuperBlockKeys = keyof Curriculum;

// eslint-disable-next-line @typescript-eslint/no-unused-vars
interface Block<T> {
  challenges: ChallengeNode['challenge'][];
}

interface CurriculumProps {
  blocks: Record<string, Block<ChallengeNode['challenge'][]>>;
}

export function getChallenges() {
  const superBlockKeys = Object.keys(SuperBlocks) as SuperBlockKeys[];
  const typedCurriculum: Curriculum = curriculum;

  return superBlockKeys
    .map(key => typedCurriculum[key]?.blocks)
    .reduce((accumulator: ChallengeNode['challenge'][], superBlock) => {
      const blockKeys = Object.keys(superBlock ?? {});
      const challengesForBlock = blockKeys.map(
        key => superBlock?.[key]?.challenges ?? []
      );
      return [...accumulator, ...flatten(challengesForBlock)];
    }, []);
}


Review comment (suggestion):
```suggestion
  const typedCurriculum: Curriculum = curriculum as Curriculum;
```
It's ugly, but it'll do for now. Working with the whole curriculum is kind of a pain, though.  I think we should probably create some smaller config files with just the information we want.

Code after:
// TODO: keeping curriculum in memory is handy if we want to field requests that
// need to 'query' the curriculum, but if we force the client to handle
// redirectToCurrentChallenge and, instead, only report the current challenge id
// via the user object, then we should *not* store this so it can be garbage
// collected.
import curriculum from '../../../config/curriculum.json';
import { SuperBlocks } from '../../../config/superblocks';

type Curriculum = { [keyValue in SuperBlocks]?: CurriculumProps };
type SuperBlockKeys = keyof Curriculum;

// eslint-disable-next-line @typescript-eslint/no-unused-vars
interface Block {
  challenges: {
    id: string;
    tests?: { id?: string }[];
    challengeType: number;
  }[];
}

interface CurriculumProps {
  blocks: Record<string, Block>;
}

export function getChallenges() {
  const superBlockKeys = Object.keys(SuperBlocks) as SuperBlockKeys[];
  const typedCurriculum: Curriculum = curriculum as Curriculum;

  return superBlockKeys
    .map(key => typedCurriculum[key]?.blocks)
    .reduce((accumulator: Block['challenges'], superBlock) => {
      const blockKeys = Object.keys(superBlock ?? {});
      const challengesForBlock = blockKeys.map(
        key => superBlock?.[key]?.challenges ?? []
      );
      return [...accumulator, ...challengesForBlock.flat()];
    }, []);
}
Repository: freeCodeCamp/freeCodeCamp
File: api/src/utils/get-challenges.ts (TypeScript)

Code before:

// eslint-disable-next-line @typescript-eslint/no-unused-vars
interface Block {
  challenges: {
    id: string;
    tests?: { id?: string }[];
    challengeType: number;
  }[];
}

interface CurriculumProps {
  blocks: Record<string, Block>;
}

export function getChallenges() {
  const superBlockKeys = Object.keys(SuperBlocks) as SuperBlockKeys[];
  const typedCurriculum: Curriculum = curriculum as Curriculum;

  return superBlockKeys
    .map(key => typedCurriculum[key]?.blocks)
    .reduce((accumulator: Block['challenges'], superBlock) => {
      const blockKeys = Object.keys(superBlock ?? {});
      const challengesForBlock = blockKeys.map(
        key => superBlock?.[key]?.challenges ?? []
      );
      return [...accumulator, ...flatten(challengesForBlock)];
    }, []);
}


Review comment (suggestion):
Can I nit-pick this - seems silly to use lodash for an in-built method:

```suggestion
      return [...accumulator, ...challengesForBlock.flat()];
```

_depth of 1 should be enough, if the type is correct_

Code after:
interface Block {
  challenges: {
    id: string;
    tests?: { id?: string }[];
    challengeType: number;
  }[];
}

interface CurriculumProps {
  blocks: Record<string, Block>;
}

export function getChallenges() {
  const superBlockKeys = Object.keys(SuperBlocks) as SuperBlockKeys[];
  const typedCurriculum: Curriculum = curriculum as Curriculum;

  return superBlockKeys
    .map(key => typedCurriculum[key]?.blocks)
    .reduce((accumulator: Block['challenges'], superBlock) => {
      const blockKeys = Object.keys(superBlock ?? {});
      const challengesForBlock = blockKeys.map(
        key => superBlock?.[key]?.challenges ?? []
      );
      return [...accumulator, ...challengesForBlock.flat()];
    }, []);
}
Repository: freeCodeCamp/freeCodeCamp
File: api/src/utils/common-challenge-functions.ts (TypeScript)

Code before:

  if (
    timezone &&
    timezone !== 'UTC' &&
    !userTimezone &&
    userTimezone === 'UTC'
  ) {
    timezone = userTimezone;
    await fastify.prisma.user.update({
      where: { id: user.id },
      data: {
        timezone
      }
    });
  }

  // Doesnt make sense as if false it wont be executed and when true sets "true" again
  // if (needsModeration) {
  //     await fastify.prisma.user.update({
  //         where: { id: user.id },
  //         data: {
  //             needsModeration: true
  //         }
  //     });
  // }

  await fastify.prisma.user.update({
    where: { id: user.id },
    data: {
      completedChallenges: userCompletedChallenges,
      needsModeration,
      savedChallenges: userSavedChallenges,
      progressTimestamps: userProgressTimestamps,
      partiallyCompletedChallenges: updatedPartiallyCompletedChallenges
    }
  });

  const updateData = {};

  return {
    alreadyCompleted,
    updateData, // Might need to remove this variable as we're updating user object in this function now instead of in the endpoint handler
    completedDate: finalChallenge.completedDate,
    userSavedChallenges
  };
}


Review comment (suggestion):
Stupidly, we do need this, because the `user` object passed to `buildUserUpdate` is mutated with: `user.needsModeration = true`.

Then, in order to set it on the DB, we need to actually perform an update

Code after:

  if (
    timezone &&
    timezone !== 'UTC' &&
    !userTimezone &&
    userTimezone === 'UTC'
  ) {
    timezone = userTimezone;
    await fastify.prisma.user.update({
      where: { id: user.id },
      data: {
        timezone
      }
    });
  }

  if (needsModeration) {
    await fastify.prisma.user.update({
      where: { id: user.id },
      data: {
        needsModeration: true
      }
    });
  }

  await fastify.prisma.user.update({
    where: { id: user.id },
    data: {
      completedChallenges: userCompletedChallenges,
      needsModeration,
      savedChallenges: userSavedChallenges,
      progressTimestamps: userProgressTimestamps,
      partiallyCompletedChallenges: updatedPartiallyCompletedChallenges
    }
  });

  const updateData = {};

  return {
    alreadyCompleted,
    updateData, // Might need to remove this variable as we're updating user object in this function now instead of in the endpoint handler
    completedDate: finalChallenge.completedDate,
    userSavedChallenges
  };
}
Repository: freeCodeCamp/freeCodeCamp
File: client/src/templates/Challenges/exam/show.tsx (TypeScript)

Code before:

      if (!isPrerequisiteCompleted) {
        qualifiedForExam = false;
        incompleteChallenge = prerequisiteChallenge.title;
        break;
      }
    }

    if (!qualifiedForExam) {
      createFlashMessage({
        type: 'danger',
        message: FlashMessages.MissingPrerequisite,
        variables: { challengeTitle: incompleteChallenge }
      });
    } else {
      // qualified to take the exam
      // TODO: show loader
      // TODO: fetch exam from server/database
      const newExam = this.state.generatedExam.map(q => {
        return { question: q.question, answer: null };
      });

      this.timerInterval = setInterval(() => {
        this.setState({
          examTimeInSeconds: this.state.examTimeInSeconds + 1
        });
      }, 1000);

      this.setState(
        {
          userExam: newExam
        },
        this.props.startExam
      );

      window.addEventListener('beforeunload', this.stopWindowClose);
      window.addEventListener('unload', this.stopWindowClose);
      window.addEventListener('popstate', this.stopBrowserBack);
    }
  };

  selectAnswer = (index: number, option: string): void => {
    const newExam = Array.from(this.state.userExam);
    newExam[index].answer = option;
    this.setState({
      userExam: newExam
    });
  };

  goToPreviousQuestion = () => {
    this.setState({

Review comment (suggestion):
```suggestion
        this.setState(state => ({
          examTimeInSeconds: state.examTimeInSeconds + 1
        }));
```
Since state updates are async, there's a risk that this.state is out of date by the time setState is called. The React docs talk about it [here](https://legacy.reactjs.org/docs/state-and-lifecycle.html#state-updates-may-be-asynchronous)

Code after:
    window.addEventListener('popstate', this.stopBrowserBack);
  };

  selectAnswer = (index: number, option: string): void => {
    const newExam = Array.from(this.state.userExam);
    newExam[index].answer = option;
    this.setState({
      userExam: newExam
    });
  };

  goToPreviousQuestion = () => {
    this.setState({
      currentQuestionIndex: this.state.currentQuestionIndex - 1
    });
  };

  goToNextQuestion = () => {
    this.setState({
      currentQuestionIndex: this.state.currentQuestionIndex + 1
    });
  };

  // TODO: have server check the exam
  createExamResults = () => {
    // TODO: show loader
    const { setExamResults } = this.props;
    const { userExam, examTimeInSeconds } = this.state;

    const results: ExamResultQuestion[] = [];

    userExam.forEach(userQuestion => {
      const questionInDb = examInDatabase.find(
        dbQuestion => userQuestion.question === dbQuestion.question
      );

      const questionResult = {
        question: userQuestion.question,
        answer: userQuestion.answer,
        correct: questionInDb?.correctAnswer === userQuestion.answer
      };

      results.push(questionResult as ExamResultQuestion);
    });

    const examResults = {
      timeInSeconds: examTimeInSeconds,
      results
    };

    setExamResults(examResults);
Repository: freeCodeCamp/freeCodeCamp
File: client/src/pages/404.tsx (TypeScript)

Code before:
import { Router } from '@reach/router';
import { withPrefix } from 'gatsby';
import React from 'react';

/* eslint-disable max-len */
import ShowProfileOrFourOhFour from '../client-only-routes/show-profile-or-four-oh-four';
import FourOhFour from '../components/FourOhFour';
/* eslint-enable max-len */

function FourOhFourPage(): JSX.Element {
  return (
    <Router>
      {/* Error from installing @types/react-helmet and @types/react-redux */}
      {/* eslint-disable-next-line @typescript-eslint/ban-ts-comment */}
      {/* @ts-ignore */}
      <ShowProfileOrFourOhFour path={withPrefix('/:maybeUser')} />

      <FourOhFour default={true} />
    </Router>
  );
}

FourOhFourPage.displayName = 'FourOhFourPage';

export default FourOhFourPage;


Review comment (suggestion):
Since the component doesn't use the `default` prop, we should remove it here instead:

```suggestion
      <FourOhFour />
```

Code after:
import { Router } from '@reach/router';
import { withPrefix } from 'gatsby';
import React from 'react';

/* eslint-disable max-len */
import ShowProfileOrFourOhFour from '../client-only-routes/show-profile-or-four-oh-four';
import FourOhFour from '../components/FourOhFour';
/* eslint-enable max-len */

function FourOhFourPage(): JSX.Element {
  return (
    <Router>
      {/* Error from installing @types/react-helmet and @types/react-redux */}
      {/* eslint-disable-next-line @typescript-eslint/ban-ts-comment */}
      {/* @ts-ignore */}
      <ShowProfileOrFourOhFour path={withPrefix('/:maybeUser')} />

      <FourOhFour default />
    </Router>
  );
}

FourOhFourPage.displayName = 'FourOhFourPage';

export default FourOhFourPage;
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/FourOhFour/index.tsx (TypeScript)

Code before:
import { Link } from 'gatsby';
import React from 'react';
import Helmet from 'react-helmet';
import { useTranslation } from 'react-i18next';

import notFoundLogo from '../../assets/images/freeCodeCamp-404.svg';
import { randomQuote } from '../../utils/get-words';
import { Spacer } from '../helpers';

import './404.css';

interface FourOhFourProps {
  default?: boolean;
}

const FourOhFour = (_props: FourOhFourProps): JSX.Element => {
  const { t } = useTranslation();
  const quote = randomQuote();

  return (
    <div className='notfound-page-wrapper'>
      <Helmet title={t('404.page-not-found') + '| freeCodeCamp'} />
      <img alt={t('404.not-found') || ''} src={notFoundLogo} />
      <Spacer size='medium' />
      <h1>{t('404.page-not-found')}.</h1>
      <Spacer size='medium' />
      <div>
        <p>{t('404.heres-a-quote')}</p>
        <Spacer size='medium' />
        <blockquote className='quote-wrapper'>
          <p className='quote'>{quote.quote}</p>
          <p className='author'>- {quote.author}</p>
        </blockquote>
      </div>
      <Spacer size='large' />
      <Link className='btn btn-cta' to='/learn'>
        {t('buttons.view-curriculum')}
      </Link>
    </div>
  );
};

export default FourOhFour;


Review comment (suggestion):
I'm assuming you're getting a "Type 'DefaultTFuncReturn' is not assignable to type 'string | undefined'." error.

This is a global issue, and should be addressed following this method: https://www.i18next.com/overview/typescript#argument-of-type-defaulttfuncreturn-is-not-assignable-to-parameter-of-type-xyz.

Code after:
import { RouteComponentProps } from '@reach/router';
import { Link } from 'gatsby';
import React from 'react';
import Helmet from 'react-helmet';
import { useTranslation } from 'react-i18next';

import notFoundLogo from '../../assets/images/freeCodeCamp-404.svg';
import { randomQuote } from '../../utils/get-words';
import { Spacer } from '../helpers';

import './404.css';

const FourOhFour = (_props: RouteComponentProps): JSX.Element => {
  const { t } = useTranslation();
  const quote = randomQuote();
  return (
    <div className='notfound-page-wrapper'>
      <Helmet title={t('404.page-not-found') + '| freeCodeCamp'} />
      <img alt={t('404.not-found')} src={notFoundLogo} />
      <Spacer size='medium' />
      <h1>{t('404.page-not-found')}.</h1>
      <Spacer size='medium' />
      <div>
        <p>{t('404.heres-a-quote')}</p>
        <Spacer size='medium' />
        <blockquote className='quote-wrapper'>
          <p className='quote'>{quote.quote}</p>
          <p className='author'>- {quote.author}</p>
        </blockquote>
      </div>
      <Spacer size='large' />
      <Link className='btn btn-cta' to='/learn'>
        {t('buttons.view-curriculum')}
      </Link>
    </div>
  );
};

export default FourOhFour;
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/create-redirect.ts (TypeScript)

Code before:
import { navigate } from 'gatsby';

const createRedirect =
  (to = '/'): (() => JSX.Element | null) =>
  () => {
    if (typeof window !== 'undefined') {
      void navigate(to);
    }
    return null;
  };

export default createRedirect;


Review comment (suggestion):
```suggestion
import { navigate } from 'gatsby';
import type { RouteComponentProps } from "@reach/router"
```

Code after:
import { navigate } from 'gatsby';
import type { RouteComponentProps } from '@reach/router';

const createRedirect =
  (to = '/'): ((_props: RouteComponentProps) => JSX.Element | null) =>
  () => {
    if (typeof window !== 'undefined') {
      void navigate(to);
    }
    return null;
  };

export default createRedirect;
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/create-redirect.ts (TypeScript)

Code before:
import { navigate } from 'gatsby';

const createRedirect =
  (to = '/'): (() => JSX.Element | null) =>
  () => {
    if (typeof window !== 'undefined') {
      void navigate(to);
    }
    return null;
  };

export default createRedirect;


Review comment (suggestion):
```suggestion
  (to = '/'): ((_props: RouteComponentProps) => JSX.Element | null) =>
```
This makes it a little clearer what the props are and why they are not used directly.

Code after:
import { navigate } from 'gatsby';
import type { RouteComponentProps } from '@reach/router';

const createRedirect =
  (to = '/'): ((_props: RouteComponentProps) => JSX.Element | null) =>
  () => {
    if (typeof window !== 'undefined') {
      void navigate(to);
    }
    return null;
  };

export default createRedirect;
Repository: freeCodeCamp/freeCodeCamp
File: client/src/pages/certification.tsx (TypeScript)

Code before:
import { Router } from '@reach/router';
import { withPrefix } from 'gatsby';
import React from 'react';

import ShowCertification from '../client-only-routes/show-certification';
import RedirectHome from '../components/redirect-home';

import './certification.css';

function Certification(): JSX.Element {
  return (
    <Router>
      <ShowCertification
        // Error from installing @types/react-helmet and @types/react-redux
        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
        // @ts-ignore
        path={withPrefix('/certification/:username/:certSlug')}
      />

      <RedirectHome />
    </Router>
  );
}

export default Certification;


Review comment (suggestion):
```suggestion
      <RedirectHome default />
```
We have to pass this prop for the component to be rendered. But `default={true}` is a little ugly and this seems nicer :shrug:

Code after:
import { Router } from '@reach/router';
import { withPrefix } from 'gatsby';
import React from 'react';

import ShowCertification from '../client-only-routes/show-certification';
import RedirectHome from '../components/redirect-home';

import './certification.css';

function Certification(): JSX.Element {
  return (
    <Router>
      <ShowCertification
        // Error from installing @types/react-helmet and @types/react-redux
        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
        // @ts-ignore
        path={withPrefix('/certification/:username/:certSlug')}
      />

      <RedirectHome default />
    </Router>
  );
}

export default Certification;
Repository: freeCodeCamp/freeCodeCamp
File: client/src/pages/settings.tsx (TypeScript)

Code before:
import { Router } from '@reach/router';
import { withPrefix } from 'gatsby';
import React from 'react';

import ShowSettings from '../client-only-routes/show-settings';
import RedirectHome from '../components/redirect-home';

function Settings(): JSX.Element {
  return (
    <Router>
      <ShowSettings path={withPrefix('/settings')} />

      <RedirectHome />
    </Router>
  );
}

Settings.displayName = 'Settings';

export default Settings;


Review comment (suggestion):
```suggestion
      <RedirectHome default />
```

Code after:
import { Router } from '@reach/router';
import { withPrefix } from 'gatsby';
import React from 'react';

import ShowSettings from '../client-only-routes/show-settings';
import RedirectHome from '../components/redirect-home';

function Settings(): JSX.Element {
  return (
    <Router>
      <ShowSettings path={withPrefix('/settings')} />

      <RedirectHome default />
    </Router>
  );
}

Settings.displayName = 'Settings';

export default Settings;