ronantakizawa/codereview-bench huggingface dataset:
Repository: freeCodeCamp/freeCodeCamp
File: client/src/templates/Challenges/ms-trophy/show.tsx (TypeScript)

Code before:
  const container = useRef<HTMLElement>(null);
  const {
    challengeMounted,
    data: {
      challengeNode: {
        challenge: {
          fields: { tests },
          title,
          challengeType,
          helpCategory
        }
      }
    },
    pageContext: { challengeMeta },
    initTests,
    updateChallengeMeta
  } = props;
  initTests(tests);
  updateChallengeMeta({
    ...challengeMeta,
    title,
    challengeType,
    helpCategory
  });
  challengeMounted(challengeMeta.id);
  container.current?.focus();
  useEffect(() => {
    const {
      challengeMounted,
      data: {
        challengeNode: {
          challenge: {
            fields: { tests },
            title,
            challengeType,
            helpCategory
          }
        }
      },
      pageContext: { challengeMeta },
      initTests,
      updateChallengeMeta
    } = props;
    initTests(tests);
    updateChallengeMeta({
      ...challengeMeta,
      title,
      challengeType,
      helpCategory
    });
    challengeMounted(challengeMeta.id);

Review comment (suggestion):
```suggestion
```
There's no need to call these on every render. This should map to the original `componentDidMount` callback, i.e. get called once when the component is first created.

Code after:
  const container = useRef<HTMLElement>(null);
  const {
    data: {
      challengeNode: {
        challenge: { title }
      }
    }
  } = props;
  useEffect(() => {
    const {
      challengeMounted,
      data: {
        challengeNode: {
          challenge: {
            fields: { tests },
            title,
            challengeType,
            helpCategory
          }
        }
      },
      pageContext: { challengeMeta },
      initTests,
      updateChallengeMeta
    } = props;
    initTests(tests);
    updateChallengeMeta({
      ...challengeMeta,
      title,
      challengeType,
      helpCategory
    });
    challengeMounted(challengeMeta.id);
    container.current?.focus();
    // This effect should be run once on mount
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);

  const handleSubmit = () => {
    const { setIsProcessing, submitChallenge } = props;

    setIsProcessing(true);
    submitChallenge();
  };

  const {
    data: {
      challengeNode: {
        challenge: {
          description,
          instructions,
Repository: freeCodeCamp/freeCodeCamp
File: client/src/templates/Challenges/ms-trophy/show.tsx (TypeScript)

Code before:
    const {
      challengeMounted,
      data: {
        challengeNode: {
          challenge: {
            fields: { tests },
            title,
            challengeType,
            helpCategory
          }
        }
      },
      pageContext: { challengeMeta },
      initTests,
      updateChallengeMeta
    } = props;
    initTests(tests);
    updateChallengeMeta({
      ...challengeMeta,
      title,
      challengeType,
      helpCategory
    });
    challengeMounted(challengeMeta.id);
    container.current?.focus();
  }, [props]);

  const handleSubmit = () => {
    const { setIsProcessing, submitChallenge } = props;

    setIsProcessing(true);
    submitChallenge();
  };

  const {
    data: {
      challengeNode: {
        challenge: {
          description,
          instructions,
          superBlock,
          block,
          translationPending,
          fields: { blockName }
        }
      }
    },
    isChallengeCompleted,
    isProcessing,
    msUsername,
    openHelpModal,

Review comment (suggestion):
```suggestion
    // This effect should be run once on mount
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);
```

Code after:
      ...challengeMeta,
      title,
      challengeType,
      helpCategory
    });
    challengeMounted(challengeMeta.id);
    container.current?.focus();
    // This effect should be run once on mount
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);

  const handleSubmit = () => {
    const { setIsProcessing, submitChallenge } = props;

    setIsProcessing(true);
    submitChallenge();
  };

  const {
    data: {
      challengeNode: {
        challenge: {
          description,
          instructions,
          superBlock,
          block,
          translationPending,
          fields: { blockName }
        }
      }
    },
    isChallengeCompleted,
    isProcessing,
    msUsername,
    openHelpModal,
    pageContext: {
      challengeMeta: { nextChallengePath, prevChallengePath }
    },
    t
  } = props;

  const blockNameTitle = `${t(
    `intro:${superBlock}.blocks.${block}.title`
  )} - ${title}`;

  return (
    <Hotkeys
      containerRef={container}
      nextChallengePath={nextChallengePath}
      prevChallengePath={prevChallengePath}
    >
Repository: freeCodeCamp/freeCodeCamp
File: client/src/templates/Challenges/projects/frontend/show.tsx (TypeScript)

Code before:
  };

  const container = useRef<HTMLElement>(null);

  useEffect(() => {
    const {
      challengeMounted,
      data: {
        challengeNode: {
          challenge: { fields, title, challengeType, helpCategory }
        }
      },
      pageContext: { challengeMeta },
      initTests,
      updateChallengeMeta
    } = props;
    initTests(fields.tests);
    updateChallengeMeta({
      ...challengeMeta,
      title,
      challengeType,
      helpCategory
    });
    challengeMounted(challengeMeta.id);
    container.current?.focus();
  }, [props]);

  const {
    data: {
      challengeNode: {
        challenge: {
          challengeType,
          fields: { blockName },
          forumTopicId,
          title,
          description,
          instructions,
          superBlock,
          block,
          translationPending
        }
      }
    },
    isChallengeCompleted,
    pageContext: {
      challengeMeta: { nextChallengePath, prevChallengePath }
    },
    t,
    updateSolutionFormValues
  } = props;


Review comment (suggestion):
```suggestion
    // This effect should be run once on mount
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);
```
It should never happen that the props change, so we don't need to include them as deps. Also, if they _did_ change, that would cause the hotkeys to receive focus (which we don't want).

Code after:
  };

  const container = useRef<HTMLElement>(null);

  useEffect(() => {
    const {
      challengeMounted,
      data: {
        challengeNode: {
          challenge: { fields, title, challengeType, helpCategory }
        }
      },
      pageContext: { challengeMeta },
      initTests,
      updateChallengeMeta
    } = props;
    initTests(fields.tests);
    updateChallengeMeta({
      ...challengeMeta,
      title,
      challengeType,
      helpCategory
    });
    challengeMounted(challengeMeta.id);
    container.current?.focus();
    // This effect should be run once on mount
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);

  const {
    data: {
      challengeNode: {
        challenge: {
          challengeType,
          fields: { blockName },
          forumTopicId,
          title,
          description,
          instructions,
          superBlock,
          block,
          translationPending
        }
      }
    },
    isChallengeCompleted,
    pageContext: {
      challengeMeta: { nextChallengePath, prevChallengePath }
    },
    t,
    updateSolutionFormValues
Repository: freeCodeCamp/freeCodeCamp
File: e2e/portfolio.spec.ts (TypeScript)

Code before:
import { execSync } from 'child_process';
import { test, expect } from '@playwright/test';
import translations from '../client/i18n/locales/english/translations.json';

test.use({ storageState: 'playwright/.auth/development-user.json' });

test.beforeAll(() => {
  execSync('node ./tools/scripts/seed/seed-demo-user');
});

test.afterAll(() => {
  execSync('node ./tools/scripts/seed/seed-demo-user --certified-user');
});

test.describe('Add Portfolio Item', () => {
  test.beforeEach(async ({ page }) => {
    test.setTimeout(61000);
    await page.goto('/developmentuser');

    if (!process.env.CI) {
      await page
        .getByRole('button', { name: 'Preview custom 404 page' })
        .click();
    }

    await page.getByRole('button', { name: 'Edit my profile' }).click();

    await page
      .getByRole('button', { name: 'Add a new portfolio Item' })
      .click();

    // Will check if the portfolio button is hydrated correctly with different intervals.
    await expect(async () => {
      await expect(
        page.getByRole('button', { name: 'Add a new portfolio Item' })
      ).toBeDisabled();
    }).toPass();
  });

  test('The title has validation', async ({ page }) => {
    await page.getByLabel(translations.settings.labels.title).fill('T');
    await expect(page.getByTestId('title-validation')).toContainText(

Review comment (suggestion):
```suggestion
```
Is this necessary? It's a very long time for a test.

Code after:
import { execSync } from 'child_process';
import { test, expect } from '@playwright/test';
import translations from '../client/i18n/locales/english/translations.json';

test.use({ storageState: 'playwright/.auth/development-user.json' });

test.beforeAll(() => {
  execSync('node ./tools/scripts/seed/seed-demo-user');
});

test.afterAll(() => {
  execSync('node ./tools/scripts/seed/seed-demo-user --certified-user');
});

test.describe('Add Portfolio Item', () => {
  test.beforeEach(async ({ page }) => {
    await page.goto('/developmentuser');

    if (!process.env.CI) {
      await page
        .getByRole('button', { name: 'Preview custom 404 page' })
        .click();
    }

    await page.getByRole('button', { name: 'Edit my profile' }).click();

    // Will check if the portfolio button is hydrated correctly with different intervals.
    await expect(async () => {
      const addPortfolioItemButton = page.getByRole('button', {
        name: 'Add a new portfolio Item'
      });
      await addPortfolioItemButton.click();

      await expect(addPortfolioItemButton).toBeDisabled({ timeout: 1 });
    }).toPass();
  });

  test('The title has validation', async ({ page }) => {
    await page.getByLabel(translations.settings.labels.title).fill('T');
    await expect(page.getByTestId('title-validation')).toContainText(
      'Title is too short'
    );
Repository: freeCodeCamp/freeCodeCamp
File: client/src/templates/Introduction/super-block-intro.tsx (TypeScript)

Code before:
      resetExpansion,
      toggleBlock: b => toggleBlock(b)
    },
    dispatch
  );

const SuperBlockIntroductionPage = (props: SuperBlockProps) => {
  const { t } = useTranslation();
  useEffect(() => {
    initializeExpandedState();
    props.tryToShowDonationModal();

    setTimeout(() => {
      configureAnchors({ offset: -40, scrollDuration: 400 });
    }, 0);

    return () => {
      configureAnchors({ offset: -40, scrollDuration: 0 });
    };
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);

  const {
    data: {
      allChallengeNode: { nodes }
    },
    isSignedIn,
    signInLoading,
    user,
    pageContext: { superBlock, title, certification },
    location,
    user: { completedChallenges: allCompletedChallenges }
  } = props;

  const allChallenges = useMemo(
    () => nodes.map(({ challenge }) => challenge),
    [nodes]
  );
  const challenges = useMemo(
    () => allChallenges.filter(c => c.superBlock === superBlock),
    [allChallenges, superBlock]
  );
  const blocks = uniq(challenges.map(({ block }) => block));

  const completedChallenges = useMemo(
    () =>
      allCompletedChallenges.filter(completedChallenge =>
        challenges.some(c => c.id === completedChallenge.id)
      ),
    [challenges, allCompletedChallenges]
  );

Review comment (suggestion):
Initially the `getChosenBlock` function is declared above `allChallenges`, `challenges`, etc. 

I want to use those variables inside the function, so what's changed here is I moved the function a little further down.

Code after:
  bindActionCreators(
    {
      tryToShowDonationModal,
      resetExpansion,
      toggleBlock: b => toggleBlock(b)
    },
    dispatch
  );

const SuperBlockIntroductionPage = (props: SuperBlockProps) => {
  const { t } = useTranslation();
  useEffect(() => {
    initializeExpandedState();
    props.tryToShowDonationModal();

    setTimeout(() => {
      configureAnchors({ offset: -40, scrollDuration: 400 });
    }, 0);

    return () => {
      configureAnchors({ offset: -40, scrollDuration: 0 });
    };
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);

  const {
    data: {
      allChallengeNode: { nodes }
    },
    isSignedIn,
    currentChallengeId,
    signInLoading,
    user,
    pageContext: { superBlock, title, certification },
    location,
    user: { completedChallenges: allCompletedChallenges }
  } = props;

  const allChallenges = useMemo(
    () => nodes.map(({ challenge }) => challenge),
    [nodes]
  );
  const superBlockChallenges = useMemo(
    () => allChallenges.filter(c => c.superBlock === superBlock),
    [allChallenges, superBlock]
  );
  const blocks = uniq(superBlockChallenges.map(({ block }) => block));

  const completedChallenges = useMemo(
    () =>
      allCompletedChallenges.filter(completedChallenge =>
Repository: freeCodeCamp/freeCodeCamp
File: e2e/super-block-page.spec.ts (TypeScript)

Code before:
import { execSync } from 'child_process';
import { expect, test } from '@playwright/test';

import { clearEditor, focusEditor, getEditors } from './utils/editor';

test.describe('Super Block Page - Authenticated User', () => {
  test.use({ storageState: 'playwright/.auth/development-user.json' });

  test.beforeEach(() => {
    execSync('node ./tools/scripts/seed/seed-demo-user');
  });

  test.afterAll(() => {
    execSync('node ./tools/scripts/seed/seed-demo-user --certified-user');
  });

  test.describe('Super Block in List View', () => {
    test('should expand the correct block when user goes to the page from breadcrumb click', async ({
      page
    }) => {
      await page.goto(
        '/learn/javascript-algorithms-and-data-structures-v8/learn-basic-javascript-by-building-a-role-playing-game/step-2'
      );

      await page
        .getByRole('link', {
          name: 'Learn Basic JavaScript by Building a Role Playing Game'
        })
        .click();

Review comment (suggestion):
I'm adding tests for both the existing behavior as well as the new behavior.

Just to aid the review process, here are the test cases:

<img width="649" alt="Screenshot 2024-12-14 at 00 45 12" src="https://github.com/user-attachments/assets/df048d43-747c-4ecd-82b5-e1efac5bfc9b" />


Code after:
import { execSync } from 'child_process';
import { expect, test } from '@playwright/test';

test.describe('Super Block Page - Authenticated User', () => {
  test.use({ storageState: 'playwright/.auth/development-user.json' });

  test.beforeEach(() => {
    execSync('node ./tools/scripts/seed/seed-demo-user');
  });

  test.afterAll(() => {
    execSync('node ./tools/scripts/seed/seed-demo-user --certified-user');
  });

  test.describe('Super Block in List View', () => {
    test('should expand the correct block when user goes to the page from breadcrumb click', async ({
      page
    }) => {
      await page.goto(
        '/learn/javascript-algorithms-and-data-structures-v8/learn-basic-javascript-by-building-a-role-playing-game/step-2'
      );

      await page
        .getByRole('link', {
          name: 'Learn Basic JavaScript by Building a Role Playing Game'
        })
        .click();

      await page.waitForURL(
Repository: freeCodeCamp/freeCodeCamp
File: client/src/templates/Introduction/super-block-intro.tsx (TypeScript)

Code before:
    }, 0);

    return () => {
      configureAnchors({ offset: -40, scrollDuration: 0 });
    };
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);

  const {
    data: {
      allChallengeNode: { nodes }
    },
    isSignedIn,
    currentChallengeId,
    signInLoading,
    user,
    pageContext: { superBlock, title, certification },
    location,
    user: { completedChallenges: allCompletedChallenges }
  } = props;

  const allChallenges = useMemo(
    () => nodes.map(({ challenge }) => challenge),
    [nodes]
  );
  const challenges = useMemo(
    () => allChallenges.filter(c => c.superBlock === superBlock),
    [allChallenges, superBlock]
  );
  const blocks = uniq(challenges.map(({ block }) => block));

  const completedChallenges = useMemo(
    () =>
      allCompletedChallenges.filter(completedChallenge =>
        challenges.some(c => c.id === completedChallenge.id)
      ),
    [challenges, allCompletedChallenges]
  );

  const i18nTitle = getSuperBlockTitleForMap(superBlock);

  const superblockWithoutCert = [
    SuperBlocks.RespWebDesign,
    SuperBlocks.CodingInterviewPrep,
    SuperBlocks.TheOdinProject,
    SuperBlocks.ProjectEuler,
    SuperBlocks.A2English,
    SuperBlocks.RosettaCode,
    SuperBlocks.PythonForEverybody
  ];


Review comment (suggestion):
```suggestion
  const superBlockChallenges = useMemo(
```
There are a lot of types of challenges! I think it's helpful to make it self-descriptive.

Code after:
      configureAnchors({ offset: -40, scrollDuration: 400 });
    }, 0);

    return () => {
      configureAnchors({ offset: -40, scrollDuration: 0 });
    };
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);

  const {
    data: {
      allChallengeNode: { nodes }
    },
    isSignedIn,
    currentChallengeId,
    signInLoading,
    user,
    pageContext: { superBlock, title, certification },
    location,
    user: { completedChallenges: allCompletedChallenges }
  } = props;

  const allChallenges = useMemo(
    () => nodes.map(({ challenge }) => challenge),
    [nodes]
  );
  const superBlockChallenges = useMemo(
    () => allChallenges.filter(c => c.superBlock === superBlock),
    [allChallenges, superBlock]
  );
  const blocks = uniq(superBlockChallenges.map(({ block }) => block));

  const completedChallenges = useMemo(
    () =>
      allCompletedChallenges.filter(completedChallenge =>
        superBlockChallenges.some(c => c.id === completedChallenge.id)
      ),
    [superBlockChallenges, allCompletedChallenges]
  );

  const i18nTitle = getSuperBlockTitleForMap(superBlock);

  const showCertification = liveCerts.some(
    cert => superBlockToCertMap[superBlock] === cert.certSlug
  );

  const superBlockWithAccordionView = [SuperBlocks.FullStackDeveloper];

  const getChosenBlock = (): string => {
    // if coming from breadcrumb click
    if (
Repository: freeCodeCamp/freeCodeCamp
File: client/src/templates/Introduction/super-block-intro.tsx (TypeScript)

Code before:
      const dashedBlock = location.hash.replace('#', '').replace('/', '');
      return dashedBlock;
    }

    if (isSignedIn) {
      // see if currentChallenge is in this superBlock
      const currentChallenge = challenges.find(
        challenge => challenge.id === currentChallengeId
      );

      if (currentChallenge) return currentChallenge.block;

      // If the current challenge isn't in the super block
      // Find the most recently completed challenge of the super block,
      // which is the last item of the `completedChallenges` array.
      if (!isEmpty(completedChallenges)) {
        const lastCompletedChallengeId = last(completedChallenges)?.id;

        const lastCompletedChallenge = nodes.find(
          node => node.challenge.id === lastCompletedChallengeId
        )?.challenge;

        if (lastCompletedChallenge) return lastCompletedChallenge.block;
      }

      return blocks[0];
    }

    return blocks[0];
  };

  const initializeExpandedState = () => {
    const { resetExpansion, toggleBlock } = props;

    resetExpansion();
    return toggleBlock(getChosenBlock());
  };

  const chosenBlock = getChosenBlock();

  const onCertificationDonationAlertClick = () => {
    callGA({
      event: 'donation_related',
      action: `Certification Donation Alert Click`
    });
  };

  return (
    <>
      <Helmet>
        <title>{i18nTitle} | freeCodeCamp.org</title>

Review comment (suggestion):
```suggestion
```
If we get to the end of the block it returns `blocks[0]` anyway.

Code after:
      // see if currentChallenge is in this superBlock
      const currentChallenge = superBlockChallenges.find(
        challenge => challenge.id === currentChallengeId
      );

      if (currentChallenge) return currentChallenge.block;

      // If the current challenge isn't in the super block
      // Find the most recently completed challenge of the super block,
      // which is the last item of the `completedChallenges` array.
      if (!isEmpty(completedChallenges)) {
        const lastCompletedChallengeId = last(completedChallenges)?.id;

        const lastCompletedChallenge = allChallenges.find(
          ({ id }) => id === lastCompletedChallengeId
        );

        if (lastCompletedChallenge) return lastCompletedChallenge.block;
      }
    }

    return blocks[0];
  };

  const initializeExpandedState = () => {
    const { resetExpansion, toggleBlock } = props;

    resetExpansion();
    return toggleBlock(getChosenBlock());
  };

  const chosenBlock = getChosenBlock();

  const onCertificationDonationAlertClick = () => {
    callGA({
      event: 'donation_related',
      action: `Certification Donation Alert Click`
    });
  };

  return (
    <>
      <Helmet>
        <title>{i18nTitle} | freeCodeCamp.org</title>
      </Helmet>
      <Container>
        <main>
          <Row className='super-block-intro-page'>
            <Col md={8} mdOffset={2} sm={10} smOffset={1} xs={12}>
              <Spacer size='l' />
              <LegacyLinks superBlock={superBlock} />