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

Code before:

  test.afterAll(async () => {
    await page.close();
  });

  test('should render the button, menu and the three links when video is available', async () => {
    await testHelpButton(page, true);
  });
});

test.describe('help-button tests for a page with two links when video is not available', () => {
  let page: Page;
  // visit the page without the video link
  test.beforeAll(async ({ browser }) => {
    page = await browser.newPage();
    await page.goto(
      '/learn/front-end-development-libraries/bootstrap/apply-the-default-bootstrap-button-style'
    );
  });

  test.afterAll(async () => {
    await page.close();
  });

  test('should render the button, menu and the two links when video is not available', async () => {
    await testHelpButton(page, false);
  });
});


Review comment (suggestion):
```suggestion
    await page.getByTestId('get-help-dropdown').click();
    await expect(page.getByTestId('get-hint')).toBeVisible();
    await expect(page.getByTestId('ask-for-help')).toBeVisible();
    await expect(page.getByTestId('watch-a-video')).toBeHidden();
```

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

test.describe('help-button tests for a page with three links (hint, help and video)', () => {
  test('should render the button, menu and the three links when video is available', async ({
    page
  }) => {
    // visit the page with the video link
    await page.goto(
      '/learn/responsive-web-design/basic-html-and-html5/say-hello-to-html-elements'
    );
    //The button is visible
    const helpButton = page.getByTestId('get-help-dropdown');
    await expect(helpButton).toBeVisible();
    //The button is clickable
    await helpButton.click();
    //The menu items are visible
    await expect(page.getByTestId('get-hint')).toBeVisible();
    await expect(page.getByTestId('ask-for-help')).toBeVisible();
    await expect(page.getByTestId('watch-a-video')).toBeVisible();
  });
});

test.describe('help-button tests for a page with two links when video is not available', () => {
  test('should render the button, menu and the two links when video is not available', async ({
    page
  }) => {
    // visit the page without the video link
    await page.goto(
      '/learn/front-end-development-libraries/bootstrap/apply-the-default-bootstrap-button-style'
    );
    //The button is visible
    const helpButton = page.getByTestId('get-help-dropdown');
    await expect(helpButton).toBeVisible();
    //The button is clickable
    await helpButton.click();
    //The menu items are visible
    await expect(page.getByTestId('get-hint')).toBeVisible();
    await expect(page.getByTestId('ask-for-help')).toBeVisible();
    //The video link is hidden
    await expect(page.getByTestId('watch-a-video')).toBeHidden();
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: client/src/templates/Challenges/rechallenge/transformers.js (JavaScript)

Code before:
  attempt,
  cond,
  flow,
  identity,
  isError,
  matchesProperty,
  overSome,
  partial,
  stubTrue
} from 'lodash-es';

import sassData from '../../../../../client/config/browser-scripts/sass-compile.json';
import {
  transformContents,
  transformHeadTailAndContents,
  setExt,
  compileHeadTail
} from '../../../../../shared/utils/polyvinyl';
import createWorker from '../utils/worker-executor';
import { makeCancellable, makeInputAwaitable } from './transform-python';

const { filename: sassCompile } = sassData;

const protectTimeout = 100;
const testProtectTimeout = 1500;
const loopsPerTimeoutCheck = 20;
const testLoopsPerTimeoutCheck = 2000;

function loopProtectCB(line) {
  console.log(
    `Potential infinite loop detected on line ${line}. Tests may fail if this is not changed.`
  );
}

function testLoopProtectCB(line) {
  console.log(
    `Potential infinite loop detected on line ${line}. Tests may be failing because of this.`
  );
}

// hold Babel and presets so we don't try to import them multiple times

let Babel;
let presetEnv, presetReact;
let presetsJS, presetsJSX;

async function loadBabel() {
  if (Babel) return;
  /* eslint-disable no-inline-comments */
  Babel = await import(
    /* webpackChunkName: "@babel/standalone" */ '@babel/standalone'

Review comment (suggestion):
```suggestion
const loopsPerTimeoutCheck = 100;
```

Code after:
  attempt,
  cond,
  flow,
  identity,
  isError,
  matchesProperty,
  overSome,
  partial,
  stubTrue
} from 'lodash-es';

import sassData from '../../../../../client/config/browser-scripts/sass-compile.json';
import {
  transformContents,
  transformHeadTailAndContents,
  setExt,
  compileHeadTail
} from '../../../../../shared/utils/polyvinyl';
import createWorker from '../utils/worker-executor';
import { makeCancellable, makeInputAwaitable } from './transform-python';

const { filename: sassCompile } = sassData;

const protectTimeout = 100;
const testProtectTimeout = 1500;
const loopsPerTimeoutCheck = 100;
const testLoopsPerTimeoutCheck = 2000;

function loopProtectCB(line) {
  console.log(
    `Potential infinite loop detected on line ${line}. Tests may fail if this is not changed.`
  );
}

function testLoopProtectCB(line) {
  console.log(
    `Potential infinite loop detected on line ${line}. Tests may be failing because of this.`
  );
}

// hold Babel and presets so we don't try to import them multiple times

let Babel;
let presetEnv, presetReact;
let presetsJS, presetsJSX;

async function loadBabel() {
  if (Babel) return;
  /* eslint-disable no-inline-comments */
  Babel = await import(
    /* webpackChunkName: "@babel/standalone" */ '@babel/standalone'
Repository: freeCodeCamp/freeCodeCamp
File: api/src/db/prisma.ts (TypeScript)

Code before:
import fp from 'fastify-plugin';
import { FastifyPluginAsync } from 'fastify';
import { PrismaClient } from '@prisma/client';

import { FREECODECAMP_NODE_ENV, MONGOHQ_URL } from '../utils/env';

declare module 'fastify' {
  interface FastifyInstance {
    prisma: PrismaClient;
  }
}

// Appends the dbId to the existing database name
const createTestConnectionURL = (url: string, dbId: string) =>
  url.replace(/(.*)(\?.*)/, `$1${dbId}$2`);

const isTest = (workerId: string | undefined): workerId is string =>
  !!workerId && FREECODECAMP_NODE_ENV === 'development';

const prismaPlugin: FastifyPluginAsync = fp(async (server, _options) => {
  if (isTest(process.env.JEST_WORKER_ID)) {
    // push the schema to the test db to setup indexes, unique constraints, etc
    execSync('pnpm prisma db push', {
      env: {
        ...process.env,
        // eslint-disable-next-line @typescript-eslint/naming-convention
        MONGOHQ_URL: createTestConnectionURL(
          MONGOHQ_URL,
          process.env.JEST_WORKER_ID
        )
      }
    });
  }

  const prisma = isTest(process.env.JEST_WORKER_ID)
    ? new PrismaClient({
        datasources: {
          db: {
            url: createTestConnectionURL(
              MONGOHQ_URL,
              process.env.JEST_WORKER_ID
            )
          }
        }
      })
    : new PrismaClient();

  await prisma.$connect();

  server.decorate('prisma', prisma);


Review comment (suggestion):
https://github.com/freeCodeCamp/freeCodeCamp/pull/51685, disabling naming convention is merged, this can be removed after rebase with main

```suggestion
```

Code after:
import fp from 'fastify-plugin';
import { FastifyPluginAsync } from 'fastify';
import { PrismaClient } from '@prisma/client';

import { FREECODECAMP_NODE_ENV, MONGOHQ_URL } from '../utils/env';

declare module 'fastify' {
  interface FastifyInstance {
    prisma: PrismaClient;
  }
}

// Appends the dbId to the existing database name
const createTestConnectionURL = (url: string, dbId: string) =>
  url.replace(/(.*)(\?.*)/, `$1${dbId}$2`);

const isTest = (workerId: string | undefined): workerId is string =>
  !!workerId && FREECODECAMP_NODE_ENV === 'development';

const prismaPlugin: FastifyPluginAsync = fp(async (server, _options) => {
  if (isTest(process.env.JEST_WORKER_ID)) {
    // push the schema to the test db to setup indexes, unique constraints, etc
    execSync('pnpm prisma db push', {
      env: {
        ...process.env,
        MONGOHQ_URL: createTestConnectionURL(
          MONGOHQ_URL,
          process.env.JEST_WORKER_ID
        )
      }
    });
  }

  const prisma = isTest(process.env.JEST_WORKER_ID)
    ? new PrismaClient({
        datasources: {
          db: {
            url: createTestConnectionURL(
              MONGOHQ_URL,
              process.env.JEST_WORKER_ID
            )
          }
        }
      })
    : new PrismaClient();

  await prisma.$connect();

  server.decorate('prisma', prisma);

  server.addHook('onClose', async server => {
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/Flash/index.tsx (TypeScript)

Code before:
import { Alert } from '@freecodecamp/ui';
import { AlertProps } from '@freecodecamp/ui/types/src/alert';
import { CloseButton } from '@freecodecamp/ui/src/close-button';
import React from 'react';
import { useTranslation } from 'react-i18next';
import { TransitionGroup, CSSTransition } from 'react-transition-group';
import { FlashState } from '../../redux/types';
import { removeFlashMessage } from './redux';

import './flash.css';

type FlashProps = {
  flashMessage: FlashState['message'];
  removeFlashMessage: typeof removeFlashMessage;
};

function Flash({ flashMessage, removeFlashMessage }: FlashProps): JSX.Element {
  const { type, message, id, variables } = flashMessage;
  const { t } = useTranslation();

  const flashStyle = type as AlertProps['variant'];

  function handleClose() {
    removeFlashMessage();
  }

  return (
    <TransitionGroup>

Review comment (suggestion):
To follow the same import style we usually have
```suggestion
import { Alert, CloseButton, type AlertProps  } from '@freecodecamp/ui';
```

---

~~btw, @ahmaxed, is it possible to add eslint rule so the import only from `@freecodecamp/ui` in another PR?~~

Code after:
import { Alert, CloseButton, type AlertProps } from '@freecodecamp/ui';
import React from 'react';
import { useTranslation } from 'react-i18next';
import { TransitionGroup, CSSTransition } from 'react-transition-group';
import { FlashState } from '../../redux/types';
import { removeFlashMessage } from './redux';

import './flash.css';

type FlashProps = {
  flashMessage: FlashState['message'];
  removeFlashMessage: typeof removeFlashMessage;
};

function Flash({ flashMessage, removeFlashMessage }: FlashProps): JSX.Element {
  const { type, message, id, variables } = flashMessage;
  const { t } = useTranslation();

  const flashStyle = type as AlertProps['variant'];

  function handleClose() {
    removeFlashMessage();
  }

  return (
    <TransitionGroup>
      <CSSTransition classNames='flash-message' key={id} timeout={500}>
        <Alert variant={flashStyle} className='flash-message'>
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/Flash/index.tsx (TypeScript)

Code before:
import { FlashState } from '../../redux/types';
import { removeFlashMessage } from './redux';

import './flash.css';

type FlashProps = {
  flashMessage: FlashState['message'];
  removeFlashMessage: typeof removeFlashMessage;
};

function Flash({ flashMessage, removeFlashMessage }: FlashProps): JSX.Element {
  const { type, message, id, variables } = flashMessage;
  const { t } = useTranslation();

  const flashStyle = type as AlertProps['variant'];

  function handleClose() {
    removeFlashMessage();
  }

  return (
    <TransitionGroup>
      <CSSTransition classNames='flash-message' key={id} timeout={500}>
        <Alert variant={flashStyle} className='flash-message'>
          {t(message, variables)}
          <CloseButton onClick={handleClose} className='close' />
        </Alert>
      </CSSTransition>
    </TransitionGroup>
  );
}

Flash.displayName = 'FlashMessages';

export default Flash;


Review comment (suggestion):
```suggestion
          <CloseButton
            onClick={handleClose}
            label={t('buttons.close')}
            className='close'
          />
```

Code after:

import './flash.css';

type FlashProps = {
  flashMessage: FlashState['message'];
  removeFlashMessage: typeof removeFlashMessage;
};

function Flash({ flashMessage, removeFlashMessage }: FlashProps): JSX.Element {
  const { type, message, id, variables } = flashMessage;
  const { t } = useTranslation();

  const flashStyle = type as AlertProps['variant'];

  function handleClose() {
    removeFlashMessage();
  }

  return (
    <TransitionGroup>
      <CSSTransition classNames='flash-message' key={id} timeout={500}>
        <Alert variant={flashStyle} className='flash-message'>
          {t(message, variables)}
          <CloseButton
            onClick={handleClose}
            label={t('buttons.close')}
            className='close'
          />
        </Alert>
      </CSSTransition>
    </TransitionGroup>
  );
}

Flash.displayName = 'FlashMessages';

export default Flash;
Repository: freeCodeCamp/freeCodeCamp
File: tools/ui-components/src/index.ts (TypeScript)

Code before:
// Use this file as the entry point for component export
// export { Button } from './button';
export { Alert } from './alert';
// export { Image } from './image';
export { Table } from './table';
// export { ToggleButton } from './toggle-button';
export { Dropdown } from './drop-down';
export { MenuItem } from './drop-down/menu-item';
export { Container } from './container';
export { Tabs, TabsList, TabsTrigger, TabsContent } from './tabs';


Review comment (suggestion):
```suggestion
export { Alert, type AlertProps } from './alert';
export { CloseButton } from './close-button';
```

Code after:
// Use this file as the entry point for component export
// export { Button } from './button';
export { Alert, type AlertProps } from './alert';
export { CloseButton } from './close-button';
// export { Image } from './image';
export { Table } from './table';
// export { ToggleButton } from './toggle-button';
export { Dropdown } from './drop-down';
export { MenuItem } from './drop-down/menu-item';
export { Container } from './container';
export { Tabs, TabsList, TabsTrigger, TabsContent } from './tabs';
Repository: freeCodeCamp/freeCodeCamp
File: e2e/template-challenges-show.spec.ts (TypeScript)

Code before:

  test('should display a success dialog when the user submits the form, and they have completed the quiz and the assignments correctly', async () => {
    // Tick the assignment box
    await page.getByRole('checkbox').check();

    // Find and click the third quiz option, which is the correct answer.
    // Set `force: true` to bypass Playwright's check
    // as the radio is visually hidden and the click event is intercepted by the `span` element.
    const quizOptions = await page.getByRole('radio').all();
    await quizOptions[2].check({ force: true });

    await page
      .getByRole('button', { name: translations.buttons['check-answer'] })
      .click();

    await expect(
      page.getByText(translations.learn['assignment-not-complete'])
    ).not.toBeVisible();

    await expect(
      page.getByText(translations.learn['wrong-answer'])
    ).not.toBeVisible();

    // There are two elements with the `dialog` role in the DOM.
    // This appears to be semantically incorrect and should be resolved
    // once we have migrated the component to use Dialog from the `ui-components` library.
    expect(page.getByRole('dialog').all()).toBeTruthy();
    await expect(
      page.getByRole('button', { name: translations.buttons['go-to-next'] })
    ).toBeVisible();
  });
});


Review comment (bug):
We have two issues here:
- Nested `dialog`
- Dialogs don't have accessible name

<img width="1680" alt="Screenshot 2023-10-01 at 23 58 13" src="https://github.com/freeCodeCamp/freeCodeCamp/assets/25715018/e0ef28db-e210-4051-94a3-4de286b92d09">

These aren't the accessibility issue I mentioned in the PR description, but I discovered them as I was debugging the test. The dialog component in this case is a Bootstrap one, so any attempt to fix the a11y issue here is a throw-away work. I'll help with the component library and the migration instead 🙂 


Code after:
  }) => {
    // Tick the assignment box
    await page.getByRole('checkbox').check();

    // Find and click the third quiz option, which is the correct answer.
    // Set `force: true` to bypass Playwright's check
    // as the radio is visually hidden and the click event is intercepted by the `span` element.
    const quizOptions = await page.getByRole('radio').all();
    await quizOptions[2].check({ force: true });

    await page
      .getByRole('button', { name: translations.buttons['check-answer'] })
      .click();

    await expect(
      page.getByText(translations.learn['assignment-not-complete'])
    ).not.toBeVisible();

    await expect(
      page.getByText(translations.learn['wrong-answer'])
    ).not.toBeVisible();

    // There are two elements with the `dialog` role in the DOM.
    // This appears to be semantically incorrect and should be resolved
    // once we have migrated the component to use Dialog from the `ui-components` library.
    expect(page.getByRole('dialog').all()).toBeTruthy();
    await expect(
      page.getByRole('button', { name: translations.buttons['go-to-next'] })
    ).toBeVisible();
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: e2e/template-challenges-show.spec.ts (TypeScript)

Code before:
    // Tick the assignment box
    await page.getByRole('checkbox').check();

    // Find and click the third quiz option, which is the correct answer.
    // Set `force: true` to bypass Playwright's check
    // as the radio is visually hidden and the click event is intercepted by the `span` element.
    const quizOptions = await page.getByRole('radio').all();
    await quizOptions[2].check({ force: true });

    await page
      .getByRole('button', { name: translations.buttons['check-answer'] })
      .click();

    await expect(
      page.getByText(translations.learn['assignment-not-complete'])
    ).not.toBeVisible();

    await expect(
      page.getByText(translations.learn['wrong-answer'])
    ).not.toBeVisible();

    // There are two elements with the `dialog` role in the DOM.
    // This appears to be semantically incorrect and should be resolved
    // once we have migrated the component to use Dialog from the `ui-components` library.
    expect(page.getByRole('dialog').all()).toBeTruthy();
    await expect(
      page.getByRole('button', { name: translations.buttons['go-to-next'] })
    ).toBeVisible();
  });
});


Review comment (bug):
This is probably minor, but I find it a little strange.

We do have a login step in Playwright setup, but the test would fail if we look for a button whose text is `submit-and-go`.

https://github.com/freeCodeCamp/freeCodeCamp/blob/22b31e10fea2075fcb713c86d02280f1caff16bd/client/src/templates/Challenges/components/completion-modal.tsx#L205

Code after:
    await page.getByRole('checkbox').check();

    // Find and click the third quiz option, which is the correct answer.
    // Set `force: true` to bypass Playwright's check
    // as the radio is visually hidden and the click event is intercepted by the `span` element.
    const quizOptions = await page.getByRole('radio').all();
    await quizOptions[2].check({ force: true });

    await page
      .getByRole('button', { name: translations.buttons['check-answer'] })
      .click();

    await expect(
      page.getByText(translations.learn['assignment-not-complete'])
    ).not.toBeVisible();

    await expect(
      page.getByText(translations.learn['wrong-answer'])
    ).not.toBeVisible();

    // There are two elements with the `dialog` role in the DOM.
    // This appears to be semantically incorrect and should be resolved
    // once we have migrated the component to use Dialog from the `ui-components` library.
    expect(page.getByRole('dialog').all()).toBeTruthy();
    await expect(
      page.getByRole('button', { name: translations.buttons['go-to-next'] })
    ).toBeVisible();
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: e2e/email-settings.spec.ts (TypeScript)

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

const settingsPageElement = {
  emailSettingsSectionHeader: 'email-settings-header',
  emailVerificationAlert: 'email-verification-alert',
  emailVerificationLink: 'email-verification-link',
  currentEmailText: 'current-email',
  newEmailInput: 'new-email-input',
  confirmEmailInput: 'confirm-email-input',
  saveButton: 'save-email-button',
  emailSubscriptionYesPleaseButton: 'yes-please-button',
  emailSubscriptionNoThanksButton: 'no-thanks-button',
  flashMessageAlert: 'flash-message'
} as const;

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

test.beforeEach(async ({ page }) => {
  await page.goto('/settings');
});

test.afterEach(async ({ page }) => {
  await page.close();
});

test.describe('Email Settings', () => {
  test('should display email settings section header on settings page', async ({
    page
  }) => {
    await expect(
      page.getByTestId(settingsPageElement.emailSettingsSectionHeader)
    ).toHaveText('Email Settings');
  });

  test('should display current email address', async ({ page }) => {
    await expect(
      page.getByTestId(settingsPageElement.currentEmailText)
    ).toHaveText('foo@bar.com');
  });

  test('should display email verification alert after email update', async ({
    page
  }) => {
    const newEmailAddress = 'foo-update@bar.com';

    await page

Review comment (suggestion):
```suggestion
test.beforeAll(async ({ browser }) => {
  page = await browser.newPage();
  await page.goto('/');
});
```

replace this with the already being used beforEeach block

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

const settingsPageElement = {
  emailSettingsSectionHeader: 'email-settings-header',
  emailVerificationAlert: 'email-verification-alert',
  emailVerificationLink: 'email-verification-link',
  currentEmailText: 'current-email',
  newEmailInput: 'new-email-input',
  confirmEmailInput: 'confirm-email-input',
  saveButton: 'save-email-button',
  emailSubscriptionYesPleaseButton: 'yes-please-button',
  emailSubscriptionNoThanksButton: 'no-thanks-button',
  flashMessageAlert: 'flash-message'
} as const;

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

test.beforeEach(async ({ page }) => {
  await page.goto('/settings');
});

test.afterEach(async ({ page }) => {
  await page.close();
});

test.describe('Email Settings', () => {
  test('should display email settings section header on settings page', async ({
    page
  }) => {
    await expect(
      page.getByTestId(settingsPageElement.emailSettingsSectionHeader)
    ).toHaveText('Email Settings');
  });

  test('should display current email address', async ({ page }) => {
    await expect(
      page.getByTestId(settingsPageElement.currentEmailText)
    ).toHaveText('foo@bar.com');
  });

  test('should display email verification alert after email update', async ({
    page,
    browserName
  }) => {
    test.skip(browserName === 'webkit', 'csrf_token cookie is being deleted');