ronantakizawa/codereview-bench huggingface dataset:
Repository: freeCodeCamp/freeCodeCamp
File: e2e/legacy-editor.spec.ts (TypeScript)

Code before:
import { test, expect } from '@playwright/test';
import { focusEditor } from './utils/editor';

test.describe('Editor Shortcuts', () => {
  test('Should handle Alt+Enter', async ({ page, isMobile }) => {
    await page.goto(
      'learn/responsive-web-design/basic-html-and-html5/say-hello-to-html-elements'
    );
    await focusEditor({ page, isMobile });

    await page.keyboard.press('Control+Enter');
    await expect(
      page.getByTestId('editor-container-indexhtml').getByText('<h1>Hello</h1>')
    ).toBeVisible();
  });

  test('Should ignore Ctrl+Enter', async ({ page, isMobile }) => {
    await page.goto(
      'learn/responsive-web-design/basic-html-and-html5/say-hello-to-html-elements'
    );
    await focusEditor({ page, isMobile });

    await page.keyboard.press('Control+Enter');
    await expect(
      page.getByTestId('editor-container-indexhtml').getByText('<h1>Hello</h1>')
    ).toBeVisible();
  });
});


Review comment (suggestion):
With Alt + Enter, I think we would want to check if there is a new line added to the code

```suggestion
      page.getByTestId('editor-container-indexhtml').getByText('<h1>Hello</h1>\n')
```

Code after:
import { test, expect } from '@playwright/test';
import { focusEditor } from './utils/editor';

test.describe('Editor Shortcuts', () => {
  test('Should add a new line if the user presses Alt+Enter', async ({
    page,
    isMobile
  }) => {
    await page.goto(
      'learn/responsive-web-design/basic-html-and-html5/say-hello-to-html-elements'
    );
    await focusEditor({ page, isMobile });

    await page.keyboard.press('Alt+Enter');
    await expect(
      page
        .getByTestId('editor-container-indexhtml')
        .getByText('<h1>Hello</h1>\n')
    ).toBeVisible();
  });

  test('Should not add a new line if the user presses Ctrl+Enter', async ({
    page,
    isMobile
  }) => {
    await page.goto(
      'learn/responsive-web-design/basic-html-and-html5/say-hello-to-html-elements'
    );
    await focusEditor({ page, isMobile });

    await page.keyboard.press('Control+Enter');
    await expect(
      page.getByTestId('editor-container-indexhtml').getByText('<h1>Hello</h1>')
    ).toBeVisible();
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: e2e/multifile-cert-projects.spec.ts (TypeScript)

Code before:

    await page.reload();

    await expect(page.getByText('save1text')).toBeVisible();
  });

  test('should save using ctrl+s hotkey and persist through navigation', async ({
    page,
    isMobile
  }) => {
    await focusEditor({ page, isMobile });

    if (isMacOS) {
      await page.keyboard.press('Meta+A');
    } else {
      await page.keyboard.press('Control+A');
    }

    await page.keyboard.type('save2text');
    await expect(page.getByText('save2text')).toBeVisible();

    await page.keyboard.down('Control');
    await page.keyboard.press('KeyS');

    await expect(page.getByTestId('flash-message')).toContainText(
      /Your code was saved to the database. It will be here when you return./
    );

    await page.getByRole('button', { name: 'Close' }).click();

    await expect(page.getByText('save2text')).toBeVisible();

    await page.reload();

    await expect(page.getByText('save2text')).toBeVisible();

    await focusEditor({ page, isMobile });

    await page.keyboard.down('Control');
    await page.keyboard.press('KeyS');

    await expect(page.getByTestId('flash-message')).toContainText(
      /Slow Down! Your code was not saved. Try again in a few seconds./
    );
  });
});


Review comment (suggestion):
```suggestion
      /Your code was saved to the database\. It will be here when you return\./
```

Code after:

    await page.reload();

    await expect(page.getByText('save1text')).toBeVisible();
  });

  test('should save using ctrl+s hotkey and persist through navigation', async ({
    page,
    isMobile
  }) => {
    await focusEditor({ page, isMobile });

    if (isMacOS) {
      await page.keyboard.press('Meta+A');
    } else {
      await page.keyboard.press('Control+A');
    }

    await page.keyboard.type('save2text');
    await expect(page.getByText('save2text')).toBeVisible();

    await page.keyboard.down('Control');
    await page.keyboard.press('KeyS');

    await expect(page.getByTestId('flash-message')).toContainText(
      /Your code was saved to the database\. It will be here when you return\./
    );

    await page.getByRole('button', { name: 'Close' }).click();

    await expect(page.getByText('save2text')).toBeVisible();

    await page.reload();

    await expect(page.getByText('save2text')).toBeVisible();

    await focusEditor({ page, isMobile });

    await page.keyboard.down('Control');
    await page.keyboard.press('KeyS');

    await expect(page.getByTestId('flash-message')).toContainText(
      /Slow Down! Your code was not saved\. Try again in a few seconds\./
    );
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: e2e/multifile-cert-projects.spec.ts (TypeScript)

Code before:

    await page.keyboard.type('save2text');
    await expect(page.getByText('save2text')).toBeVisible();

    await page.keyboard.down('Control');
    await page.keyboard.press('KeyS');

    await expect(page.getByTestId('flash-message')).toContainText(
      /Your code was saved to the database. It will be here when you return./
    );

    await page.getByRole('button', { name: 'Close' }).click();

    await expect(page.getByText('save2text')).toBeVisible();

    await page.reload();

    await expect(page.getByText('save2text')).toBeVisible();

    await focusEditor({ page, isMobile });

    await page.keyboard.down('Control');
    await page.keyboard.press('KeyS');

    await expect(page.getByTestId('flash-message')).toContainText(
      /Slow Down! Your code was not saved. Try again in a few seconds./
    );
  });
});


Review comment (performance):
```suggestion
      /Slow Down! Your code was not saved\. Try again in a few seconds\./
```

Code after:

    await page.keyboard.type('save2text');
    await expect(page.getByText('save2text')).toBeVisible();

    await page.keyboard.down('Control');
    await page.keyboard.press('KeyS');

    await expect(page.getByTestId('flash-message')).toContainText(
      /Your code was saved to the database\. It will be here when you return\./
    );

    await page.getByRole('button', { name: 'Close' }).click();

    await expect(page.getByText('save2text')).toBeVisible();

    await page.reload();

    await expect(page.getByText('save2text')).toBeVisible();

    await focusEditor({ page, isMobile });

    await page.keyboard.down('Control');
    await page.keyboard.press('KeyS');

    await expect(page.getByTestId('flash-message')).toContainText(
      /Slow Down! Your code was not saved\. Try again in a few seconds\./
    );
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: e2e/search-bar-optimized.spec.ts (TypeScript)

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

const getSearchInput = async ({
  page,
  isMobile
}: {
  page: Page;
  isMobile: boolean;
}) => {
  if (isMobile) {
    const menuButton = page.getByRole('button', {
      name: translations.buttons.menu
    });
    await expect(menuButton).toBeVisible();
    await menuButton.click();
  }

  return page.getByLabel('Search');
};

test.describe('Search bar optimized', () => {
  test.beforeEach(async ({ page }) => {
    await page.goto('/');
  });

  test('should display correct placeholder', async ({ page, isMobile }) => {
    const searchInput = await getSearchInput({ page, isMobile });

    await expect(searchInput).toBeVisible();
    await expect(searchInput).toHaveAttribute(
      'placeholder',
      'Search 10,700+ tutorials'
    );
  });

  test('should return the search results when the user presses Enter', async ({
    context,
    page,
    isMobile
  }) => {
    const searchInput = await getSearchInput({ page, isMobile });
    await expect(searchInput).toBeVisible();

    await searchInput.fill('test');
    await page.keyboard.press('Enter');

    // Wait for the new page to load.
    const newPage = await context.waitForEvent('page');

Review comment (performance):
This is how I could tell which search bar component this file is testing. 

`/` uses `SearchBarOptimized`:

https://github.com/freeCodeCamp/freeCodeCamp/blob/88fefcf87b8cc2b307de8d985722a9b17fdcfc15/client/src/components/Header/components/universal-nav.tsx#L42-L47

---

I'll create a `search-bar.spec.ts` for the other search bar. I think it's easier to maintain if they are in separate files.

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

const getSearchInput = async ({
  page,
  isMobile
}: {
  page: Page;
  isMobile: boolean;
}) => {
  if (isMobile) {
    const menuButton = page.getByRole('button', {
      name: translations.buttons.menu
    });
    await expect(menuButton).toBeVisible();
    await menuButton.click();
  }

  return page.getByLabel('Search');
};

test.describe('Search bar optimized', () => {
  test.beforeEach(async ({ page }) => {
    await page.goto('/');
  });

  test('should display correct placeholder', async ({ page, isMobile }) => {
    const searchInput = await getSearchInput({ page, isMobile });

    await expect(searchInput).toBeVisible();
    await expect(searchInput).toHaveAttribute(
      'placeholder',
      translations.search.placeholder
    );
  });

  test('should return the search results when the user presses Enter', async ({
    context,
    page,
    isMobile
  }) => {
    const searchInput = await getSearchInput({ page, isMobile });
    await expect(searchInput).toBeVisible();

    await searchInput.fill('test');
    await page.keyboard.press('Enter');

    // Wait for the new page to load.
    const newPage = await context.waitForEvent('page');
Repository: freeCodeCamp/freeCodeCamp
File: api-server/src/server/boot/randomAPIs.js (JavaScript)

Code before:
      return new Promise((resolve, reject) =>
        user.updateAttributes(
          {
            sendQuincyEmail: true
          },
          err => {
            if (err) {
              reject(err);
            } else {
              resolve();
            }
          }
        )
      )
        .then(() => {
          req.flash(
            'success',
            "We've successfully updated your email preferences. Thank you for resubscribing."
          );
          return res.redirectWithFlash(origin);
        })
        .catch(next);
    });
  }

  const blockedUserAgentParts = ['python', 'google-apps-script'];

  function blockUserAgent(req, res, next) {
    const userAgent = req.headers['user-agent'];

    if (
      !userAgent ||
      blockedUserAgentParts.some(ua => userAgent.toLowerCase().includes(ua))
    ) {
      return res.status(403).send('Forbidden');
    }

    return next();
  }

  async function getPublicProfile(req, res) {
    const { username } = req.query;
    if (!username) {
      return res.status(400).json({ error: 'No username provided' });
    }

    const user = await User.findOne({ where: { username } });

    if (!user) {
      return res.status(404).json({ error: 'User not found' });
    }

Review comment (suggestion):
```suggestion
  const blockedUserAgentParts = ['python', 'google-apps-script', 'curl'];
```

Code after:
      return new Promise((resolve, reject) =>
        user.updateAttributes(
          {
            sendQuincyEmail: true
          },
          err => {
            if (err) {
              reject(err);
            } else {
              resolve();
            }
          }
        )
      )
        .then(() => {
          req.flash(
            'success',
            "We've successfully updated your email preferences. Thank you for resubscribing."
          );
          return res.redirectWithFlash(origin);
        })
        .catch(next);
    });
  }

  const blockedUserAgentParts = ['python', 'google-apps-script', 'curl'];

  function blockUserAgent(req, res, next) {
    const userAgent = req.headers['user-agent'];

    if (
      !userAgent ||
      blockedUserAgentParts.some(ua => userAgent.toLowerCase().includes(ua))
    ) {
      return res
        .status(400)
        .send(
          'This endpoint is no longer available outside of the freeCodeCamp ecosystem'
        );
    }

    return next();
  }

  async function getPublicProfile(req, res) {
    const { username } = req.query;
    if (!username) {
      return res.status(400).json({ error: 'No username provided' });
    }

    const user = await User.findOne({ where: { username } });
Repository: freeCodeCamp/freeCodeCamp
File: api-server/src/server/boot/randomAPIs.js (JavaScript)

Code before:
              resolve();
            }
          }
        )
      )
        .then(() => {
          req.flash(
            'success',
            "We've successfully updated your email preferences. Thank you for resubscribing."
          );
          return res.redirectWithFlash(origin);
        })
        .catch(next);
    });
  }

  const blockedUserAgentParts = ['python', 'google-apps-script'];

  function blockUserAgent(req, res, next) {
    const userAgent = req.headers['user-agent'];

    if (
      !userAgent ||
      blockedUserAgentParts.some(ua => userAgent.toLowerCase().includes(ua))
    ) {
      return res.status(403).send('Forbidden');
    }

    return next();
  }

  async function getPublicProfile(req, res) {
    const { username } = req.query;
    if (!username) {
      return res.status(400).json({ error: 'No username provided' });
    }

    const user = await User.findOne({ where: { username } });

    if (!user) {
      return res.status(404).json({ error: 'User not found' });
    }

    const { completedChallenges, progressTimestamps, profileUI } = user;
    const allUser = {
      ...pick(user, publicUserProps),
      points: progressTimestamps.length,
      completedChallenges,
      ...getProgress(progressTimestamps),
      ...normaliseUserFields(user),
      joinDate: user.id.getTimestamp()

Review comment (suggestion):
```suggestion
      return res
        .status(400)
        .send(
          'This endpoint is no longer available outside of the freeCodeCamp ecosystem'
        );
```

Code after:
              resolve();
            }
          }
        )
      )
        .then(() => {
          req.flash(
            'success',
            "We've successfully updated your email preferences. Thank you for resubscribing."
          );
          return res.redirectWithFlash(origin);
        })
        .catch(next);
    });
  }

  const blockedUserAgentParts = ['python', 'google-apps-script', 'curl'];

  function blockUserAgent(req, res, next) {
    const userAgent = req.headers['user-agent'];

    if (
      !userAgent ||
      blockedUserAgentParts.some(ua => userAgent.toLowerCase().includes(ua))
    ) {
      return res
        .status(400)
        .send(
          'This endpoint is no longer available outside of the freeCodeCamp ecosystem'
        );
    }

    return next();
  }

  async function getPublicProfile(req, res) {
    const { username } = req.query;
    if (!username) {
      return res.status(400).json({ error: 'No username provided' });
    }

    const user = await User.findOne({ where: { username } });

    if (!user) {
      return res.status(404).json({ error: 'User not found' });
    }

    const { completedChallenges, progressTimestamps, profileUI } = user;
    const allUser = {
      ...pick(user, publicUserProps),
      points: progressTimestamps.length,
Repository: freeCodeCamp/freeCodeCamp
File: tools/challenge-helper-scripts/create-project.ts (TypeScript)

Code before:

// fs Promise functions return errors, but no stack trace.  This adds back in
// the stack trace.
function withTrace<Args extends unknown[], Result>(
  fn: (...x: Args) => Promise<Result>,
  ...args: Args
): Promise<Result> {
  return fn(...args).catch((reason: Error) => {
    throw Error(reason.message);
  });
}

void prompt([
  {
    name: 'superBlock',
    message: 'Which certification does this belong to?',
    default: SuperBlocks.RespWebDesign,
    type: 'list',
    choices: Object.values(SuperBlocks)
  },
  {
    name: 'block',
    message: 'What is the dashed name (in kebab-case) for this project?',
    validate: validateBlockName,
    filter: (block: string) => {
      return block.toLowerCase();
    }
  },
  {
    name: 'title',
    default: ({ block }: { block: string }) => block
  },
  {
    name: 'helpCategory',
    message: 'Choose a help category',
    default: 'HTML-CSS',
    type: 'list',
    choices: helpCategories
  },
  {
    name: 'order',
    message: 'Which position does this appear in the certificate?',
    default: 42,
    validate: (order: string) => {
      return parseInt(order, 10) > 0
        ? true
        : 'Order must be an number greater than zero.';
    },
    filter: (order: string) => {
      return parseInt(order, 10);
    }

Review comment (suggestion):
```suggestion
      return block.toLowerCase().trim();
```
If we're allowing trailing whitespace to pass validation, I think we should remove it here.

Code after:

// fs Promise functions return errors, but no stack trace.  This adds back in
// the stack trace.
function withTrace<Args extends unknown[], Result>(
  fn: (...x: Args) => Promise<Result>,
  ...args: Args
): Promise<Result> {
  return fn(...args).catch((reason: Error) => {
    throw Error(reason.message);
  });
}

void prompt([
  {
    name: 'superBlock',
    message: 'Which certification does this belong to?',
    default: SuperBlocks.RespWebDesign,
    type: 'list',
    choices: Object.values(SuperBlocks)
  },
  {
    name: 'block',
    message: 'What is the dashed name (in kebab-case) for this project?',
    validate: validateBlockName,
    filter: (block: string) => {
      return block.toLowerCase().trim();
    }
  },
  {
    name: 'title',
    default: ({ block }: { block: string }) => block
  },
  {
    name: 'helpCategory',
    message: 'Choose a help category',
    default: 'HTML-CSS',
    type: 'list',
    choices: helpCategories
  },
  {
    name: 'order',
    message: 'Which position does this appear in the certificate?',
    default: 42,
    validate: (order: string) => {
      return parseInt(order, 10) > 0
        ? true
        : 'Order must be an number greater than zero.';
    },
    filter: (order: string) => {
      return parseInt(order, 10);
    }
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/Donation/donation-modal.tsx (TypeScript)

Code before:
  const [canClose, setCanClose] = useState(false);

  useEffect(() => {
    if (show) {
      void playTone('donation');
      callGA({ event: 'pageview', pagePath: '/donation-modal' });
      callGA({
        event: 'donation_view',
        action: `Displayed ${
          recentlyClaimedBlock !== null ? 'Block' : 'Progress'
        } Donation Modal`
      });
    }
  }, [show, recentlyClaimedBlock]);

  const handleModalHide = () => {
    if (!canClose) {
      return;
    }

    // If modal is open on a SuperBlock page
    if (isLocationSuperBlock(location)) {
      goToAnchor('claim-cert-block');
    }

    closeDonationModal();
  };

  return (
    <Modal size='large' onClose={handleModalHide} open={show}>
      <DonationModalBody
        closeDonationModal={closeDonationModal}
        recentlyClaimedBlock={recentlyClaimedBlock}
        setCanClose={setCanClose}
      />
    </Modal>
  );
}

DonateModal.displayName = 'DonateModal';

export default connect(mapStateToProps, mapDispatchToProps)(DonateModal);


Review comment (suggestion):
Under the hood, `Modal` calls `onClose` when the Esc key is pressed. `handleModalHide` previously didn't trigger the close action so the keypress was ignored.

Code after:
}: DonateModalProps): JSX.Element {
  const [canClose, setCanClose] = useState(false);
  const isA11yFeatureEnabled = useFeature('a11y-donation-modal').on;

  useEffect(() => {
    if (show) {
      void playTone('donation');
      callGA({ event: 'pageview', pagePath: '/donation-modal' });
      callGA({
        event: 'donation_view',
        action: `Displayed ${
          recentlyClaimedBlock !== null ? 'Block' : 'Progress'
        } Donation Modal`
      });
    }
  }, [show, recentlyClaimedBlock]);

  const handleModalHide = () => {
    // If modal is open on a SuperBlock page
    if (isLocationSuperBlock(location)) {
      goToAnchor('claim-cert-block');
    }

    if (isA11yFeatureEnabled && canClose) {
      closeDonationModal();
    }
  };

  return (
    <Modal size='large' onClose={handleModalHide} open={show}>
      <DonationModalBody
        closeDonationModal={closeDonationModal}
        recentlyClaimedBlock={recentlyClaimedBlock}
        setCanClose={setCanClose}
      />
    </Modal>
  );
}

DonateModal.displayName = 'DonateModal';

export default connect(mapStateToProps, mapDispatchToProps)(DonateModal);
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/Donation/donation-modal.tsx (TypeScript)

Code before:
  useEffect(() => {
    if (show) {
      void playTone('donation');
      callGA({ event: 'pageview', pagePath: '/donation-modal' });
      callGA({
        event: 'donation_view',
        action: `Displayed ${
          recentlyClaimedBlock !== null ? 'Block' : 'Progress'
        } Donation Modal`
      });
    }
  }, [show, recentlyClaimedBlock]);

  const handleModalHide = () => {
    // If modal is open on a SuperBlock page
    if (isLocationSuperBlock(location)) {
      goToAnchor('claim-cert-block');
    }

    if (isA11yFeatureEnabled) {
      if (!canClose) {
        return;
      }

      closeDonationModal();
    }
  };

  return (
    <Modal size='large' onClose={handleModalHide} open={show}>
      <DonationModalBody
        closeDonationModal={closeDonationModal}
        recentlyClaimedBlock={recentlyClaimedBlock}
        setCanClose={setCanClose}
      />
    </Modal>
  );
}

DonateModal.displayName = 'DonateModal';

export default connect(mapStateToProps, mapDispatchToProps)(DonateModal);


Review comment (suggestion):

Wouldn't the following be more straightforward. 
```suggestion
    if (isA11yFeatureEnabled && canClose ) {
              closeDonationModal();
    }
```

Code after:
  useEffect(() => {
    if (show) {
      void playTone('donation');
      callGA({ event: 'pageview', pagePath: '/donation-modal' });
      callGA({
        event: 'donation_view',
        action: `Displayed ${
          recentlyClaimedBlock !== null ? 'Block' : 'Progress'
        } Donation Modal`
      });
    }
  }, [show, recentlyClaimedBlock]);

  const handleModalHide = () => {
    // If modal is open on a SuperBlock page
    if (isLocationSuperBlock(location)) {
      goToAnchor('claim-cert-block');
    }

    if (isA11yFeatureEnabled && canClose) {
      closeDonationModal();
    }
  };

  return (
    <Modal size='large' onClose={handleModalHide} open={show}>
      <DonationModalBody
        closeDonationModal={closeDonationModal}
        recentlyClaimedBlock={recentlyClaimedBlock}
        setCanClose={setCanClose}
      />
    </Modal>
  );
}

DonateModal.displayName = 'DonateModal';

export default connect(mapStateToProps, mapDispatchToProps)(DonateModal);