ronantakizawa/codereview-bench huggingface dataset:
Repository: freeCodeCamp/freeCodeCamp
File: tools/ui-components/src/container/container.test.tsx (TypeScript)

Code before:
import React from 'react';
import { render, screen } from '@testing-library/react';

import { Container } from '.';

describe('<Container />', () => {
  it('remove width when the container is fluid', () => {
    render(
      <Container fluid={true}>Random text to test the element width</Container>
    );
    expect(
      screen.getByText('Random text to test the element width')
    ).toHaveClass('mx-auto px-[15px] ');
  });
  it('should add className to it', () => {
    render(
      <Container className='certificate-outer-wrapper'>
        Random text to test the element width
      </Container>
    );
    expect(
      screen.getByText('Random text to test the element width')
    ).toHaveClass(
      'mx-auto px-[15px] my-0 md:w-[750px] min-[992px]:w-[970px] min-[1200px]:w-[1170px] certificate-outer-wrapper'
    );
  });
});


Review comment (suggestion):
```suggestion
        Learn to code for free.
```

Code after:
import React from 'react';
import { render, screen } from '@testing-library/react';

import { Container } from '.';

describe('<Container />', () => {
  it('remove width when the container is fluid', () => {
    render(<Container fluid={true}>Learn to code for free.</Container>);
    expect(screen.getByText('Learn to code for free.')).toHaveClass(
      'mx-auto px-[15px] '
    );
  });
  it('should add className to it', () => {
    render(
      <Container className='certificate-outer-wrapper'>
        Learn to code for free.
      </Container>
    );
    expect(screen.getByText('Learn to code for free.')).toHaveClass(
      'mx-auto px-[15px] my-0 md:w-[750px] min-[992px]:w-[970px] min-[1200px]:w-[1170px] certificate-outer-wrapper'
    );
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: tools/ui-components/src/container/container.test.tsx (TypeScript)

Code before:
import React from 'react';
import { render, screen } from '@testing-library/react';

import { Container } from '.';

describe('<Container />', () => {
  it('remove width when the container is fluid', () => {
    render(
      <Container fluid={true}>Learn to code for free.</Container>
    );
    expect(
      screen.getByText('Random text to test the element width')
    ).toHaveClass('mx-auto px-[15px] ');
  });
  it('should add className to it', () => {
    render(
      <Container className='certificate-outer-wrapper'>
        Learn to code for free.
      </Container>
    );
    expect(
      screen.getByText('Random text to test the element width')
    ).toHaveClass(
      'mx-auto px-[15px] my-0 md:w-[750px] min-[992px]:w-[970px] min-[1200px]:w-[1170px] certificate-outer-wrapper'
    );
  });
});


Review comment (suggestion):
```suggestion
      screen.getByText('Learn to code for free.')
```

Code after:
import React from 'react';
import { render, screen } from '@testing-library/react';

import { Container } from '.';

describe('<Container />', () => {
  it('remove width when the container is fluid', () => {
    render(<Container fluid={true}>Learn to code for free.</Container>);
    expect(screen.getByText('Learn to code for free.')).toHaveClass(
      'mx-auto px-[15px] '
    );
  });
  it('should add className to it', () => {
    render(
      <Container className='certificate-outer-wrapper'>
        Learn to code for free.
      </Container>
    );
    expect(screen.getByText('Learn to code for free.')).toHaveClass(
      'mx-auto px-[15px] my-0 md:w-[750px] min-[992px]:w-[970px] min-[1200px]:w-[1170px] certificate-outer-wrapper'
    );
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: tools/ui-components/src/row/row.tsx (TypeScript)

Code before:
import React from 'react';

import { RowProps } from './types';

export const Row = ({ className, children }: RowProps) => {
  if (!className) {
    className = '';
  }
  return <div className={`mx-[-15px] ${className}`}>{children}</div>;
};


Review comment (suggestion):
```suggestion
  return <div className={`mx-[-15px] ${className ?? ''}`}>{children}</div>;
```
Nitpick :man_shrugging: 

Code after:
import React from 'react';

import { RowProps } from './types';

export const Row = ({ className, children, ...rest }: RowProps) => {
  return (
    <div className={`mx-[-15px] ${className ?? ''}`} {...rest}>
      {children}
    </div>
  );
};
Repository: freeCodeCamp/freeCodeCamp
File: api/src/routes/challenge.test.ts (TypeScript)

Code before:
};

// /modern-challenge-completed
const HtmlChallengeId = '5dc174fcf86c76b9248c6eb2';
const JsProjectId = '56533eb9ac21ba0edf2244e2';
const multiFileCertProjectId = 'bd7158d8c242eddfaeb5bd13';

const HtmlChallengeBody = {
  challengeType: 0,
  id: HtmlChallengeId
};
const JsProjectBody = {
  challengeType: 5,
  id: JsProjectId,
  files: [
    {
      contents: 'console.log("Hello There!")',
      key: 'scriptjs',
      ext: 'js',
      name: 'script',
      history: ['script.js']
    }
  ]
};
const multiFileCertProjectBody = {
  challengeType: 14,
  id: multiFileCertProjectId,
  files: [
    {
      contents: '<h1>Multi File Project v1</h1>',
      key: 'indexhtml',
      ext: 'html',
      name: 'index',
      history: ['index.html']
    },
    {
      contents: '.hello-there { general: kenobi; }',
      key: 'stylescss',
      ext: 'css',
      name: 'styles',
      history: ['styles.css']
    }
  ]
};
const updatedMultiFileCertProjectBody = {
  challengeType: 14,
  id: multiFileCertProjectId,
  files: [
    {
      contents: '<h1>Multi File Project v2</h1>',
      key: 'indexhtml',

Review comment (suggestion):
```suggestion
} as const;
```
TS is only complaining below because in principle this array could be emptied. Now it can't.

Code after:
const backendChallengeBody2 = {
  id: backendChallengeId2
};

// /modern-challenge-completed
const HtmlChallengeId = '5dc174fcf86c76b9248c6eb2';
const JsProjectId = '56533eb9ac21ba0edf2244e2';
const multiFileCertProjectId = 'bd7158d8c242eddfaeb5bd13';

const HtmlChallengeBody = {
  challengeType: challengeTypes.html,
  id: HtmlChallengeId
};
const JsProjectBody = {
  challengeType: challengeTypes.jsProject,
  id: JsProjectId,
  files: [
    {
      contents: 'console.log("Hello There!")',
      key: 'scriptjs',
      ext: 'js',
      name: 'script',
      history: ['script.js']
    }
  ]
};
const multiFileCertProjectBody = {
  challengeType: challengeTypes.multifileCertProject,
  id: multiFileCertProjectId,
  files: [
    {
      contents: '<h1>Multi File Project v1</h1>',
      key: 'indexhtml',
      ext: 'html',
      name: 'index',
      history: ['index.html']
    },
    {
      contents: '.hello-there { general: kenobi; }',
      key: 'stylescss',
      ext: 'css',
      name: 'styles',
      history: ['styles.css']
    }
  ]
};
const updatedMultiFileCertProjectBody = {
  challengeType: challengeTypes.multifileCertProject,
  id: multiFileCertProjectId,
  files: [
    {
Repository: freeCodeCamp/freeCodeCamp
File: client/src/pages/email-sign-up.tsx (TypeScript)

Code before:
const mapStateToProps = createSelector(
  userSelector,
  isSignedInSelector,
  signInLoadingSelector,
  (
    {
      acceptedPrivacyTerms,
      completedChallengeCount
    }: { acceptedPrivacyTerms: boolean; completedChallengeCount: number },
    isSignedIn: boolean,
    showLoading: boolean
  ) => ({
    acceptedPrivacyTerms,
    isSignedIn,
    showLoading,
    completedChallengeCount
  })
);
const mapDispatchToProps = (dispatch: Dispatch) =>
  bindActionCreators({ acceptTerms }, dispatch);
const RedirectToLearn = createRedirect('/learn');

function EmailListOptIn({
  isSignedIn,
  acceptTerms,
  t
}: {
  isSignedIn: boolean;
  acceptTerms: (accepted: boolean) => void;
  t: TFunction;
}) {
  if (isSignedIn) {
    return (
      <Row>
        <Col md={4} mdOffset={2} sm={5} smOffset={1} xs={12}>
          <Button
            block={true}
            bsSize='lg'
            bsStyle='primary'
            className='big-cta-btn'
            onClick={() => acceptTerms(true)}
          >
            {t('buttons.yes-please')}
          </Button>
          <Spacer size='small' />
        </Col>
        <Col md={4} sm={5} xs={12}>
          <Button
            block={true}
            bsSize='lg'
            bsStyle='primary'

Review comment (refactor):
Could we also drop this prop to simplify things a bit more? 😄 We can grab the `t` function from the `useTranslation` hook.

Code after:
  isSignedInSelector,
  signInLoadingSelector,
  (
    {
      acceptedPrivacyTerms,
      completedChallengeCount
    }: { acceptedPrivacyTerms: boolean; completedChallengeCount: number },
    isSignedIn: boolean,
    showLoading: boolean
  ) => ({
    acceptedPrivacyTerms,
    isSignedIn,
    showLoading,
    completedChallengeCount
  })
);
const mapDispatchToProps = (dispatch: Dispatch) =>
  bindActionCreators({ acceptTerms }, dispatch);
const RedirectToLearn = createRedirect('/learn');

function EmailListOptIn({
  isSignedIn,
  acceptTerms
}: {
  isSignedIn: boolean;
  acceptTerms: (accepted: boolean) => void;
}) {
  const { t } = useTranslation();
  if (isSignedIn) {
    return (
      <Row>
        <Col md={4} mdOffset={2} sm={5} smOffset={1} xs={12}>
          <Button
            block={true}
            bsSize='lg'
            bsStyle='primary'
            className='big-cta-btn'
            onClick={() => acceptTerms(true)}
          >
            {t('buttons.yes-please')}
          </Button>
          <Spacer size='small' />
        </Col>
        <Col md={4} sm={5} xs={12}>
          <Button
            block={true}
            bsSize='lg'
            bsStyle='primary'
            className='big-cta-btn'
            onClick={() => acceptTerms(false)}
          >
Repository: freeCodeCamp/freeCodeCamp
File: api/src/utils/get-challenges.ts (TypeScript)

Code before:
// 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.values(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()];
    }, []);
}


Review comment (suggestion):
```suggestion
  const superBlockKeys = Object.values(SuperBlocks);
```
Sorry, I'm not 100% awake. We can get rid of the type, too.

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

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

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

export function getChallenges() {
  const superBlockKeys = Object.values(SuperBlocks);
  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-server/src/server/boot/challenge.js (JavaScript)

Code before:
    const isCompleted = completedChallenges.some(
      challenge => challenge.id === completedChallenge.id
    );

    if (!isPartiallyCompleted && !isCompleted) {
      return res.status(403).json({
        type: 'error',
        message: 'You have to complete the project before you can submit a URL.'
      });
    }
  }

  const isMSTrophyProject = completedChallenge.challengeType === 18;
  let isTrophyMissing = false;
  if (isMSTrophyProject) {
    try {
      const mSLearnAPIUrl = getApiUrlFromTrophy(completedChallenge.solution);
      isTrophyMissing = mSLearnAPIUrl ? !(await fetch(mSLearnAPIUrl)).ok : true;
    } catch {
      isTrophyMissing = true;
      log('Error verifying trophy');
    }
  }

  try {
    // This is an ugly hack to update `user.completedChallenges`
    await user.getCompletedChallenges$().toPromise();
  } catch (e) {
    return next(e);
  }

  const { alreadyCompleted, updateData } = buildUserUpdate(
    user,
    completedChallenge.id,
    completedChallenge
  );

  user.updateAttributes(updateData, err => {
    if (err) {
      return next(err);
    }

    return res.json({
      alreadyCompleted,
      points: alreadyCompleted ? user.points : user.points + 1,
      completedDate: completedChallenge.completedDate,
      ...(isMSTrophyProject && { isTrophyMissing })
    });
  });
}


Review comment (suggestion):
Just a quick comment that if all we want to do is check the URL points to an existing page, we should be able to make a HEAD request and save some bandwith.

```js
      isTrophyMissing = mSLearnAPIUrl ? !(await fetch(mSLearnAPIUrl, { method: 'HEAD'})).ok : true;
```

Code after:

    const isCompleted = completedChallenges.some(
      challenge => challenge.id === completedChallenge.id
    );

    if (!isPartiallyCompleted && !isCompleted) {
      return res.status(403).json({
        type: 'error',
        message: 'You have to complete the project before you can submit a URL.'
      });
    }
  }

  const isMSTrophyProject = completedChallenge.challengeType === 18;
  let isTrophyMissing = false;
  if (isMSTrophyProject) {
    if (!isMicrosoftLearnLink(completedChallenge.solution)) {
      return res.status(403).json({
        type: 'error',
        message:
          'You have not provided the valid links for us to inspect your work.'
      });
    }
    try {
      const mSLearnAPIUrl = getApiUrlFromTrophy(completedChallenge.solution);
      isTrophyMissing = mSLearnAPIUrl ? !(await fetch(mSLearnAPIUrl)).ok : true;
    } catch {
      isTrophyMissing = true;
      log(`Error verifying trophy: ${completedChallenge.solution}`);
    }
  }

  try {
    // This is an ugly hack to update `user.completedChallenges`
    await user.getCompletedChallenges$().toPromise();
  } catch (e) {
    return next(e);
  }

  const { alreadyCompleted, updateData } = buildUserUpdate(
    user,
    completedChallenge.id,
    completedChallenge
  );

  user.updateAttributes(updateData, err => {
    if (err) {
      return next(err);
    }

    return res.json({
Repository: freeCodeCamp/freeCodeCamp
File: api/src/routes/user.test.ts (TypeScript)

Code before:
          username: 'darth-vader',
          reportDescription: 'Luke, I am your father'
        });

        const user = await fastifyTestInstance.prisma.user.findFirstOrThrow({
          where: { email: 'foo@bar.com' }
        });

        expect(sendEmailSpy).toBeCalledTimes(1);
        expect(sendEmailSpy).toBeCalledWith({
          from: 'team@freecodecamp.org',
          to: 'support@freecodecamp.org',
          cc: user?.email,
          subject: "Abuse Report: Reporting darth-vader's profile",
          text: generateReportEmail(
            user,
            'darth-vader',
            'Luke, I am your father'
          )
        });

        expect(response.statusCode).toBe(200);
        expect(response.body).toStrictEqual({
          type: 'info',
          message: 'flash.report-sent',
          variables: { email: user?.email }
        });
      });
    });
  });

  describe('Unauthenticated user', () => {
    let setCookies: string[];
    // Get the CSRF cookies from an unprotected route
    beforeAll(async () => {
      const res = await superRequest('/', { method: 'GET' });
      setCookies = res.get('Set-Cookie');
    });

    describe('/account/delete', () => {
      test('POST returns 401 status code with error message', async () => {
        const response = await superRequest('/account/delete', {
          method: 'POST',
          setCookies
        });

        expect(response.statusCode).toBe(401);
      });
    });

    describe('/account/reset-progress', () => {

Review comment (suggestion):
```suggestion
          variables: { email: user.email }
```

Code after:


Reported by:
Username: 
Name: 
Email: foo@bar.com

Thanks and regards,
`
        });

        expect(response.statusCode).toBe(200);
        expect(response.body).toStrictEqual({
          type: 'info',
          message: 'flash.report-sent',
          variables: { email: 'foo@bar.com' }
        });
      });
    });
  });

  describe('Unauthenticated user', () => {
    let setCookies: string[];
    // Get the CSRF cookies from an unprotected route
    beforeAll(async () => {
      const res = await superRequest('/status/ping', { method: 'GET' });
      setCookies = res.get('Set-Cookie');
    });

    const endpoints: { path: string; method: 'GET' | 'POST' | 'DELETE' }[] = [
      { path: '/account/delete', method: 'POST' },
      { path: '/account/reset-progress', method: 'POST' },
      { path: '/user/get-session-user', method: 'GET' },
      { path: '/user/user-token', method: 'DELETE' },
      { path: '/user/user-token', method: 'POST' }
    ];

    endpoints.forEach(({ path, method }) => {
      test(`${method} ${path} returns 401 status code with error message`, async () => {
        const response = await superRequest(path, {
          method,
          setCookies
        });
        expect(response.statusCode).toBe(401);
      });
    });

    describe('/user/report-user', () => {
      test('POST returns 401 status code with error message', async () => {
        const response = await superRequest('/user/report-user', {
          method: 'POST',
Repository: freeCodeCamp/freeCodeCamp
File: api/src/routes/user.ts (TypeScript)

Code before:
        if (count === 0) {
          void reply.code(404);
          return {
            message: 'userToken not found',
            type: 'info'
          } as const;
        }
        return { userToken: null };
      } catch (err) {
        fastify.log.error(err);
        void reply.code(500);
        return {
          type: 'danger',
          message:
            'Oops! Something went wrong. Please try again in a moment or contact support@freecodecamp.org if the error persists.'
        } as const;
      }
    }
  );

  fastify.post(
    '/user/report-user',
    {
      schema: schemas.reportUser,
      preValidation: (req, reply, done) => {
        // NOTE: Is this the right hook?
        req.body = {
          ...req.body,
          reportDescription: trimTags(req.body.reportDescription)
        };
        done();
      }
    },
    async (req, reply) => {
      try {
        const user = await fastify.prisma.user.findUniqueOrThrow({
          where: { id: req.session.user.id }
        });
        const { username, reportDescription: report } = req.body;

        if (!username || !report || report === '') {
          // NOTE: Do we want to log these instances?
          void reply.code(400);
          return {
            type: 'danger',
            message: 'flash.provide-username'
          } as const;
        }

        await fastify.sendEmail({
          from: 'team@freecodecamp.org',

Review comment (suggestion):
```suggestion
      preHandler: (req, _reply, done) => {
```
`preHandler` makes more sense to me, since we end up with cleaner code overall.

Code after:
            isQaCertV7: true,
            isRelationalDatabaseCertV8: true,
            isRespWebDesignCert: true,
            isSciCompPyCertV7: true,
            keyboardShortcuts: true,
            linkedin: true,
            location: true,
            name: true,
            partiallyCompletedChallenges: true,
            picture: true,
            portfolio: true,
            profileUI: true,
            progressTimestamps: true,
            savedChallenges: true,
            sendQuincyEmail: true,
            theme: true,
            twitter: true,
            username: true,
            usernameDisplay: true,
            website: true,
            yearsTopContributor: true
          }
        });

        const [userToken, user] = await Promise.all([userTokenP, userP]);

        if (!user?.username) {
          void res.code(500);
          return { user: {}, result: '' };
        }

        const encodedToken = userToken
          ? encodeUserToken(userToken.id)
          : undefined;

        const {
          username,
          usernameDisplay,
          completedChallenges,
          progressTimestamps,
          twitter,
          profileUI,
          savedChallenges,
          partiallyCompletedChallenges,
          ...publicUser
        } = user;

        return {
          user: {
            [username]: {
              ...removeNulls(publicUser),