ronantakizawa/codereview-bench huggingface dataset:
Repository: freeCodeCamp/freeCodeCamp
File: api/src/exam-environment/routes/exam-environment.test.ts (TypeScript)

Code before:

        const userExam = constructUserExam(generatedExam!, mock.exam);

        expect(res).toMatchObject({
          status: 200,
          body: {
            data: {
              examAttempt,
              exam: userExam
            }
          }
        });
      });
    });

    xdescribe('POST /exam-environment/screenshot', () => {});

    describe('GET /exam-environment/exams', () => {
      it('should return 200', async () => {
        const res = await superGet('/exam-environment/exams').set(
          'exam-environment-authorization-token',
          examEnvironmentAuthorizationToken
        );
        expect(res.status).toBe(200);

        expect(res.body).toMatchObject({
          data: {
            exams: [
              {
                canTake: true,
                config: {
                  name: mock.exam.config.name,
                  note: mock.exam.config.note,
                  totalTimeInMS: mock.exam.config.totalTimeInMS
                },
                id: mock.examId
              }
            ]
          }
        });
      });
    });
  });

  describe('Authenticated user without exam environment authorization token', () => {
    let superPost: ReturnType<typeof createSuperRequest>;
    let superGet: ReturnType<typeof createSuperRequest>;

    // Authenticate user
    beforeAll(async () => {
      const setCookies = await devLogin();

Review comment (suggestion):
```suggestion
        expect(res.body).toStrictEqual({
```
Nit, strict is better, if possible.

Code after:

        const userExam = constructUserExam(generatedExam!, mock.exam);

        expect(res).toMatchObject({
          status: 200,
          body: {
            data: {
              examAttempt,
              exam: userExam
            }
          }
        });
      });
    });

    xdescribe('POST /exam-environment/screenshot', () => {});

    describe('GET /exam-environment/exams', () => {
      it('should return 200', async () => {
        const res = await superGet('/exam-environment/exams').set(
          'exam-environment-authorization-token',
          examEnvironmentAuthorizationToken
        );
        expect(res.status).toBe(200);

        expect(res.body).toStrictEqual({
          data: {
            exams: [
              {
                canTake: true,
                config: {
                  name: mock.exam.config.name,
                  note: mock.exam.config.note,
                  totalTimeInMS: mock.exam.config.totalTimeInMS
                },
                id: mock.examId
              }
            ]
          }
        });
      });
    });
  });

  describe('Authenticated user without exam environment authorization token', () => {
    let superPost: ReturnType<typeof createSuperRequest>;
    let superGet: ReturnType<typeof createSuperRequest>;

    // Authenticate user
    beforeAll(async () => {
      const setCookies = await devLogin();
Repository: freeCodeCamp/freeCodeCamp
File: api/src/exam-environment/utils/exam.ts (TypeScript)

Code before:
/* eslint-disable jsdoc/require-returns, jsdoc/require-param */
import {
  EnvAnswer,
  EnvConfig,
  EnvExam,
  EnvExamAttempt,
  EnvGeneratedExam,
  EnvMultipleChoiceQuestion,
  EnvQuestionSet,
  EnvQuestionSetAttempt
} from '@prisma/client';
import { type Static } from '@fastify/type-provider-typebox';
import * as schemas from '../schemas';

interface CompletedChallengeId {
  completedChallenges: {
    id: string;
  }[];
}

/**
 * Checks if all exam prerequisites have been met by the user.
 */
export function checkPrerequisites(
  user: CompletedChallengeId,
  prerequisites: EnvExam['prerequisites']
) {
  return prerequisites.every(p =>
    user.completedChallenges.find(c => c.id === p)
  );
}

export type UserExam = Omit<
  EnvExam,
  'questionSets' | 'config' | 'id' | 'prerequisites'
> & {
  config: Omit<EnvExam['config'], 'tags' | 'questionSets'>;
  questionSets: (Omit<EnvQuestionSet, 'questions'> & {
    questions: (Omit<
      EnvMultipleChoiceQuestion,
      'answers' | 'tags' | 'deprecated'
    > & {
      answers: Omit<EnvAnswer, 'isCorrect'>[];
    })[];
  })[];
} & { generatedExamId: string; examId: string };

/**
 * Takes the generated exam and the original exam, and creates the user-facing exam.
 */

Review comment (suggestion):
Pretty minor, but I would just pass around the array of completedChallenges. Reason being, this function doesn't need to know about the structure of `user`. We can make it (and its types) even simpler by only supplying the relevant data. 

Code after:
/* eslint-disable jsdoc/require-returns, jsdoc/require-param */
import {
  EnvAnswer,
  EnvConfig,
  EnvExam,
  EnvExamAttempt,
  EnvGeneratedExam,
  EnvMultipleChoiceQuestion,
  EnvQuestionSet,
  EnvQuestionSetAttempt
} from '@prisma/client';
import { type Static } from '@fastify/type-provider-typebox';
import * as schemas from '../schemas';

interface CompletedChallengeId {
  completedChallenges: {
    id: string;
  }[];
}

/**
 * Checks if all exam prerequisites have been met by the user.
 */
export function checkPrerequisites(
  user: CompletedChallengeId,
  prerequisites: EnvExam['prerequisites']
) {
  return prerequisites.every(p =>
    user.completedChallenges.some(c => c.id === p)
  );
}

export type UserExam = Omit<
  EnvExam,
  'questionSets' | 'config' | 'id' | 'prerequisites'
> & {
  config: Omit<EnvExam['config'], 'tags' | 'questionSets'>;
  questionSets: (Omit<EnvQuestionSet, 'questions'> & {
    questions: (Omit<
      EnvMultipleChoiceQuestion,
      'answers' | 'tags' | 'deprecated'
    > & {
      answers: Omit<EnvAnswer, 'isCorrect'>[];
    })[];
  })[];
} & { generatedExamId: string; examId: string };

/**
 * Takes the generated exam and the original exam, and creates the user-facing exam.
 */
Repository: freeCodeCamp/freeCodeCamp
File: api/src/exam-environment/utils/exam.ts (TypeScript)

Code before:
  EnvConfig,
  EnvExam,
  EnvExamAttempt,
  EnvGeneratedExam,
  EnvMultipleChoiceQuestion,
  EnvQuestionSet,
  EnvQuestionSetAttempt
} from '@prisma/client';
import { type Static } from '@fastify/type-provider-typebox';
import * as schemas from '../schemas';

interface CompletedChallengeId {
  completedChallenges: {
    id: string;
  }[];
}

/**
 * Checks if all exam prerequisites have been met by the user.
 */
export function checkPrerequisites(
  user: CompletedChallengeId,
  prerequisites: EnvExam['prerequisites']
) {
  return prerequisites.every(p =>
    user.completedChallenges.find(c => c.id === p)
  );
}

export type UserExam = Omit<
  EnvExam,
  'questionSets' | 'config' | 'id' | 'prerequisites'
> & {
  config: Omit<EnvExam['config'], 'tags' | 'questionSets'>;
  questionSets: (Omit<EnvQuestionSet, 'questions'> & {
    questions: (Omit<
      EnvMultipleChoiceQuestion,
      'answers' | 'tags' | 'deprecated'
    > & {
      answers: Omit<EnvAnswer, 'isCorrect'>[];
    })[];
  })[];
} & { generatedExamId: string; examId: string };

/**
 * Takes the generated exam and the original exam, and creates the user-facing exam.
 */
export function constructUserExam(
  generatedExam: EnvGeneratedExam,
  exam: EnvExam
): UserExam {

Review comment (suggestion):
```suggestion
    user.completedChallenges.some(c => c.id === p)
```
Slightly more direct.

Code after:
  EnvConfig,
  EnvExam,
  EnvExamAttempt,
  EnvGeneratedExam,
  EnvMultipleChoiceQuestion,
  EnvQuestionSet,
  EnvQuestionSetAttempt
} from '@prisma/client';
import { type Static } from '@fastify/type-provider-typebox';
import * as schemas from '../schemas';

interface CompletedChallengeId {
  completedChallenges: {
    id: string;
  }[];
}

/**
 * Checks if all exam prerequisites have been met by the user.
 */
export function checkPrerequisites(
  user: CompletedChallengeId,
  prerequisites: EnvExam['prerequisites']
) {
  return prerequisites.every(p =>
    user.completedChallenges.some(c => c.id === p)
  );
}

export type UserExam = Omit<
  EnvExam,
  'questionSets' | 'config' | 'id' | 'prerequisites'
> & {
  config: Omit<EnvExam['config'], 'tags' | 'questionSets'>;
  questionSets: (Omit<EnvQuestionSet, 'questions'> & {
    questions: (Omit<
      EnvMultipleChoiceQuestion,
      'answers' | 'tags' | 'deprecated'
    > & {
      answers: Omit<EnvAnswer, 'isCorrect'>[];
    })[];
  })[];
} & { generatedExamId: string; examId: string };

/**
 * Takes the generated exam and the original exam, and creates the user-facing exam.
 */
export function constructUserExam(
  generatedExam: EnvGeneratedExam,
  exam: EnvExam
): UserExam {
Repository: freeCodeCamp/freeCodeCamp
File: api/src/exam-environment/utils/exam.ts (TypeScript)

Code before:
/* eslint-disable jsdoc/require-returns, jsdoc/require-param */
import {
  EnvAnswer,
  EnvConfig,
  EnvExam,
  EnvExamAttempt,
  EnvGeneratedExam,
  EnvMultipleChoiceQuestion,
  EnvQuestionSet,
  EnvQuestionSetAttempt
} from '@prisma/client';
import { type Static } from '@fastify/type-provider-typebox';
import * as schemas from '../schemas';

interface CompletedChallengeId {
  completedChallenges: {
    id: string;
  }[];
}

/**
 * Checks if all exam prerequisites have been met by the user.
 */
export function checkPrerequisites(
  user: CompletedChallengeId,
  prerequisites: EnvExam['prerequisites']
) {
  return prerequisites.every(p =>
    user.completedChallenges.find(c => c.id === p)
  );
}

export type UserExam = Omit<
  EnvExam,
  'questionSets' | 'config' | 'id' | 'prerequisites'
> & {
  config: Omit<EnvExam['config'], 'tags' | 'questionSets'>;
  questionSets: (Omit<EnvQuestionSet, 'questions'> & {
    questions: (Omit<
      EnvMultipleChoiceQuestion,
      'answers' | 'tags' | 'deprecated'
    > & {
      answers: Omit<EnvAnswer, 'isCorrect'>[];
    })[];

Review comment (suggestion):
```suggestion
type CompletedChallenges = {
  id: string;
}[];
```
To go with the suggestion below.

Code after:
/* eslint-disable jsdoc/require-returns, jsdoc/require-param */
import {
  EnvAnswer,
  EnvConfig,
  EnvExam,
  EnvExamAttempt,
  EnvGeneratedExam,
  EnvMultipleChoiceQuestion,
  EnvQuestionSet,
  EnvQuestionSetAttempt
} from '@prisma/client';
import { type Static } from '@fastify/type-provider-typebox';
import * as schemas from '../schemas';

interface CompletedChallengeId {
  completedChallenges: {
    id: string;
  }[];
}

/**
 * Checks if all exam prerequisites have been met by the user.
 */
export function checkPrerequisites(
  user: CompletedChallengeId,
  prerequisites: EnvExam['prerequisites']
) {
  return prerequisites.every(p =>
    user.completedChallenges.some(c => c.id === p)
  );
}

export type UserExam = Omit<
  EnvExam,
  'questionSets' | 'config' | 'id' | 'prerequisites'
> & {
  config: Omit<EnvExam['config'], 'tags' | 'questionSets'>;
  questionSets: (Omit<EnvQuestionSet, 'questions'> & {
    questions: (Omit<
      EnvMultipleChoiceQuestion,
      'answers' | 'tags' | 'deprecated'
    > & {
      answers: Omit<EnvAnswer, 'isCorrect'>[];
    })[];
Repository: freeCodeCamp/freeCodeCamp
File: client/src/templates/Introduction/components/block-header.tsx (TypeScript)

Code before:
  completedCount,
  courseCompletionStatus,
  handleClick,
  isCompleted,
  isExpanded,
  percentageCompleted,
  blockIntroArr
}: BlockHeaderProps): JSX.Element {
  return (
    <h3 className='block-grid-title'>
      <button
        aria-expanded={isExpanded ? 'true' : 'false'}
        aria-controls={`${blockDashed}-panel`}
        className='block-header'
        onClick={handleClick}
      >
        <span className='block-header-button-text map-title'>
          <CheckMark isCompleted={isCompleted} />
          {blockType && <BlockLabel blockType={blockType} />}
          <span>
            {blockTitle}
            <span className='sr-only'>, {courseCompletionStatus}</span>
          </span>
          <DropDown />
        </span>
        {blockIntroArr && blockIntroArr.length && (
          <BlockIntros intros={blockIntroArr} />
        )}
        {!isExpanded && !isCompleted && completedCount > 0 && (
          <div aria-hidden='true' className='progress-wrapper'>
            <div>
              <ProgressBar now={percentageCompleted} />
            </div>
            <span>{`${percentageCompleted}%`}</span>
          </div>
        )}
      </button>
    </h3>
  );
}

export default BlockHeader;


Review comment (suggestion):
```suggestion
        {!isEmpty(blockIntroArr) && (
```
Nitpicky, I know, but this is more declarative. (btw, we need to get `isEmpty` from `lodash-es` for reasons of tree shaking).  

Code after:
  blockTitle,
  blockType,
  completedCount,
  courseCompletionStatus,
  handleClick,
  isCompleted,
  isExpanded,
  percentageCompleted,
  blockIntroArr
}: BlockHeaderProps): JSX.Element {
  return (
    <h3 className='block-grid-title'>
      <button
        aria-expanded={isExpanded ? 'true' : 'false'}
        aria-controls={`${blockDashed}-panel`}
        className='block-header'
        onClick={handleClick}
      >
        <span className='block-header-button-text map-title'>
          <CheckMark isCompleted={isCompleted} />
          {blockType && <BlockLabel blockType={blockType} />}
          <span>
            {blockTitle}
            <span className='sr-only'>, {courseCompletionStatus}</span>
          </span>
          <DropDown />
        </span>
        {!isEmpty(blockIntroArr) && (
          <BlockIntros intros={blockIntroArr as string[]} />
        )}
        {!isExpanded && !isCompleted && completedCount > 0 && (
          <div aria-hidden='true' className='progress-wrapper'>
            <div>
              <ProgressBar now={percentageCompleted} />
            </div>
            <span>{`${percentageCompleted}%`}</span>
          </div>
        )}
      </button>
    </h3>
  );
}

export default BlockHeader;
Repository: freeCodeCamp/freeCodeCamp
File: client/src/templates/Introduction/components/super-block-accordion.tsx (TypeScript)

Code before:
import React, { ReactNode, useMemo } from 'react';
import { uniq } from 'lodash-es';
import { useTranslation } from 'react-i18next';
// TODO: Add this component to freecodecamp/ui and remove this dependency
import { Disclosure } from '@headlessui/react';

import { ChallengeNode } from '../../../redux/prop-types';
import { SuperBlocks } from '../../../../../shared/config/curriculum';
import DropDown from '../../../assets/icons/dropdown';
import { BlockTypes } from '../../../../../shared/config/blocks';
import Block from './block';

import './super-block-accordion.css';

interface ChapterProps {
  dashedName: string;
  children: ReactNode;
  isExpanded: boolean;
}

interface ModuleProps {
  dashedName: string;
  children: ReactNode;
  isExpanded: boolean;
}
interface SuperBlockTreeViewProps {
  challenges: ChallengeNode['challenge'][];
  superBlock: SuperBlocks;
  chosenBlock: string;
}

Review comment (suggestion):
We can totally build the accordion from scratch, but I decided to pull in the Disclosure component from HeadlessUI so help track the expanded/collapsed state.

I've created https://github.com/freeCodeCamp/ui/issues/419 to move it to the fcc/ui library.

Code after:
import React, { ReactNode, useMemo } from 'react';
import { uniqBy } from 'lodash-es';
import { useTranslation } from 'react-i18next';
// TODO: Add this component to freecodecamp/ui and remove this dependency
import { Disclosure } from '@headlessui/react';

import { ChallengeNode } from '../../../redux/prop-types';
import { SuperBlocks } from '../../../../../shared/config/curriculum';
import DropDown from '../../../assets/icons/dropdown';
import { BlockTypes } from '../../../../../shared/config/blocks';
import Block from './block';

import './super-block-accordion.css';

interface ChapterProps {
  dashedName: string;
  children: ReactNode;
  isExpanded: boolean;
}

interface ModuleProps {
  dashedName: string;
  children: ReactNode;
  isExpanded: boolean;
}
interface SuperBlockTreeViewProps {
  challenges: ChallengeNode['challenge'][];
  superBlock: SuperBlocks;
  chosenBlock: string;
}
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/search/searchBar/search-bar-footer.tsx (TypeScript)

Code before:
import React from 'react';
import { useTranslation } from 'react-i18next';
import { searchPageUrl } from '../../../utils/algolia-locale-setup';
import NoHitsSuggestion from './no-hits-suggestion';

interface SearchBarFooterProps {
  hasHits: boolean;
  query?: string;
  handleMouseEnter: (e: React.SyntheticEvent<HTMLElement, Event>) => void;
  handleMouseLeave: (e: React.SyntheticEvent<HTMLElement, Event>) => void;
}

const SearchBarFooter = ({
  hasHits,
  query,
  handleMouseEnter,
  handleMouseLeave
}: SearchBarFooterProps) => {
  const { t } = useTranslation();
  const title = t('search.no-tutorials');

  const searchUrl = searchPageUrl;

  if (hasHits && query) {
    return (
      <li className='ais-Hits-item'>
        <a
          className='fcc_suggestion_item'
          href={searchUrl + '?query=' + encodeURIComponent(query ?? '')}
          rel='noopener noreferrer'
          target='_blank'
        >
          <span className='hit-name'>
            {t('search.see-results', { searchQuery: query })}
          </span>
        </a>
      </li>
    );
  } else if (query && !hasHits) {
    return (
      <NoHitsSuggestion
        title={title}
        handleMouseEnter={handleMouseEnter}
        handleMouseLeave={handleMouseLeave}
      />
    );
  } else {

Review comment (suggestion):
```suggestion
```
Using `searchPageUrl` directly seems fine, tbh.

Code after:
import React from 'react';
import { useTranslation } from 'react-i18next';
import { searchPageUrl } from '../../../utils/algolia-locale-setup';
import NoHitsSuggestion from './no-hits-suggestion';

interface SearchBarFooterProps {
  hasHits: boolean;
  query?: string;
  selectedIndex: number;
  onMouseEnter: (e: React.SyntheticEvent<HTMLElement, Event>) => void;
  onMouseLeave: (e: React.SyntheticEvent<HTMLElement, Event>) => void;
}

const SearchBarFooter = ({
  hasHits,
  query,
  selectedIndex,
  onMouseEnter,
  onMouseLeave
}: SearchBarFooterProps) => {
  const { t } = useTranslation();
  const title = t('search.no-tutorials');

  if (!query) {
    return null;
  }

  return hasHits ? (
    <li
      className={`ais-Hits-item ${selectedIndex === 5 ? 'selected' : ''}`}
      onMouseEnter={onMouseEnter}
      onMouseLeave={onMouseLeave}
    >
      <a
        className='fcc_suggestion_item'
        href={searchPageUrl + '?query=' + encodeURIComponent(query ?? '')}
        rel='noopener noreferrer'
        target='_blank'
      >
        <span className='hit-name'>
          {t('search.see-results', { searchQuery: query })}
        </span>
      </a>
    </li>
  ) : (
    <NoHitsSuggestion title={title} />
  );
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/search/searchBar/search-bar-footer.tsx (TypeScript)

Code before:
import React from 'react';
import { useTranslation } from 'react-i18next';
import { searchPageUrl } from '../../../utils/algolia-locale-setup';
import NoHitsSuggestion from './no-hits-suggestion';

interface SearchBarFooterProps {
  hasHits: boolean;
  query?: string;
  handleMouseEnter: (e: React.SyntheticEvent<HTMLElement, Event>) => void;
  handleMouseLeave: (e: React.SyntheticEvent<HTMLElement, Event>) => void;
}

const SearchBarFooter = ({
  hasHits,
  query,
  handleMouseEnter,
  handleMouseLeave
}: SearchBarFooterProps) => {
  const { t } = useTranslation();
  const title = t('search.no-tutorials');

  const searchUrl = searchPageUrl;

  if (hasHits && query) {
    return (
      <li className='ais-Hits-item'>
        <a
          className='fcc_suggestion_item'
          href={searchUrl + '?query=' + encodeURIComponent(query ?? '')}
          rel='noopener noreferrer'
          target='_blank'
        >
          <span className='hit-name'>
            {t('search.see-results', { searchQuery: query })}
          </span>
        </a>
      </li>
    );
  } else if (query && !hasHits) {
    return (
      <NoHitsSuggestion
        title={title}
        handleMouseEnter={handleMouseEnter}
        handleMouseLeave={handleMouseLeave}
      />
    );
  } else {
    return null;
  }

Review comment (refactor):
Since `!query` means we return `null`, you can just do that first and simplify the logic to `if (hasHits)`. That or `return hasHits ? ... : ...`



Code after:
import React from 'react';
import { useTranslation } from 'react-i18next';
import { searchPageUrl } from '../../../utils/algolia-locale-setup';
import NoHitsSuggestion from './no-hits-suggestion';

interface SearchBarFooterProps {
  hasHits: boolean;
  query?: string;
  selectedIndex: number;
  onMouseEnter: (e: React.SyntheticEvent<HTMLElement, Event>) => void;
  onMouseLeave: (e: React.SyntheticEvent<HTMLElement, Event>) => void;
}

const SearchBarFooter = ({
  hasHits,
  query,
  selectedIndex,
  onMouseEnter,
  onMouseLeave
}: SearchBarFooterProps) => {
  const { t } = useTranslation();
  const title = t('search.no-tutorials');

  if (!query) {
    return null;
  }

  return hasHits ? (
    <li
      className={`ais-Hits-item ${selectedIndex === 5 ? 'selected' : ''}`}
      onMouseEnter={onMouseEnter}
      onMouseLeave={onMouseLeave}
    >
      <a
        className='fcc_suggestion_item'
        href={searchPageUrl + '?query=' + encodeURIComponent(query ?? '')}
        rel='noopener noreferrer'
        target='_blank'
      >
        <span className='hit-name'>
          {t('search.see-results', { searchQuery: query })}
        </span>
      </a>
    </li>
  ) : (
    <NoHitsSuggestion title={title} />
  );
};
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/search/searchBar/search-bar-footer.tsx (TypeScript)

Code before:
const SearchBarFooter = ({
  hasHits,
  query,
  handleMouseEnter,
  handleMouseLeave
}: SearchBarFooterProps) => {
  const { t } = useTranslation();
  const title = t('search.no-tutorials');

  const searchUrl = searchPageUrl;

  if (hasHits && query) {
    return (
      <li className='ais-Hits-item'>
        <a
          className='fcc_suggestion_item'
          href={searchUrl + '?query=' + encodeURIComponent(query ?? '')}
          rel='noopener noreferrer'
          target='_blank'
        >
          <span className='hit-name'>
            {t('search.see-results', { searchQuery: query })}
          </span>
        </a>
      </li>
    );
  } else if (query && !hasHits) {
    return (
      <NoHitsSuggestion
        title={title}
        handleMouseEnter={handleMouseEnter}
        handleMouseLeave={handleMouseLeave}
      />
    );
  } else {
    return null;
  }
};

export default SearchBarFooter;


Review comment (suggestion):
I think this also needs to react to mouse enters and leaves. `onMouseEnter` and `onMouseLeave` should do the trick.

Code after:

const SearchBarFooter = ({
  hasHits,
  query,
  selectedIndex,
  onMouseEnter,
  onMouseLeave
}: SearchBarFooterProps) => {
  const { t } = useTranslation();
  const title = t('search.no-tutorials');

  if (!query) {
    return null;
  }

  return hasHits ? (
    <li
      className={`ais-Hits-item ${selectedIndex === 5 ? 'selected' : ''}`}
      onMouseEnter={onMouseEnter}
      onMouseLeave={onMouseLeave}
    >
      <a
        className='fcc_suggestion_item'
        href={searchPageUrl + '?query=' + encodeURIComponent(query ?? '')}
        rel='noopener noreferrer'
        target='_blank'
      >
        <span className='hit-name'>
          {t('search.see-results', { searchQuery: query })}
        </span>
      </a>
    </li>
  ) : (
    <NoHitsSuggestion title={title} />
  );
};

export default SearchBarFooter;