ronantakizawa/codereview-bench huggingface dataset:
Repository: freeCodeCamp/freeCodeCamp
File: api/src/routes/email-subscription.ts (TypeScript)

Code before:
        );
      } catch (error) {
        fastify.log.error(error);
        void reply.code(302);
        return reply.redirectWithMessage(origin, {
          type: 'danger',
          content: 'Something went wrong.'
        });
      }
    }
  );

  fastify.get(
    '/resubscribe/:unsubscribeId',
    {
      schema: schemas.resubscribe,
      errorHandler(error, request, reply) {
        if (error.validation) {
          const { origin } = getRedirectParams(request);
          void reply.code(302);
          void reply.redirectWithMessage(origin, {
            type: 'info',
            content:
              'We were unable to process this request, please check and try again.'
          });
        } else {
          fastify.errorHandler(error, request, reply);
        }
      }
    },
    async (req, reply) => {
      try {
        const { origin } = getRedirectParams(req);
        const { unsubscribeId } = req.params;
        const users = await fastify.prisma.user.findMany({
          where: { unsubscribeId }
        });

        if (!users.length) {
          void reply.code(302);
          return reply.redirectWithMessage(origin, {
            type: 'info',
            content: 'We could not find an account to resubscribe.'
          });
        }

        await fastify.prisma.user.update({
          where: { id: users[0]?.id },
          data: {
            sendQuincyEmail: true
          }

Review comment (bug):
The original string is:

> We we unable to process this request, please check and try again

We we not be fixing bugs until v1.0 of the new api.

Code after:
            content: "We've successfully updated your email preferences."
          }
        );
      } catch (error) {
        fastify.log.error(error);
        void reply.code(302);
        return reply.redirectWithMessage(origin, {
          type: 'danger',
          content: 'Something went wrong.'
        });
      }
    }
  );

  fastify.get(
    '/resubscribe/:unsubscribeId',
    {
      schema: schemas.resubscribe,
      errorHandler(error, request, reply) {
        if (error.validation) {
          const { origin } = getRedirectParams(request);
          void reply.code(302);
          void reply.redirectWithMessage(origin, {
            type: 'info',
            content:
              'We were unable to process this request, please check and try again.'
          });
        } else {
          fastify.errorHandler(error, request, reply);
        }
      }
    },
    async (req, reply) => {
      try {
        const { origin } = getRedirectParams(req);
        const { unsubscribeId } = req.params;
        const user = await fastify.prisma.user.findFirst({
          where: { unsubscribeId }
        });

        if (!user) {
          void reply.code(302);
          return reply.redirectWithMessage(origin, {
            type: 'info',
            content: 'We could not find an account to resubscribe.'
          });
        }

        await fastify.prisma.user.update({
          where: { id: user.id },
          data: {
Repository: freeCodeCamp/freeCodeCamp
File: api/src/schemas/email-subscription/resubscribe.ts (TypeScript)

Code before:
import { Type } from '@fastify/type-provider-typebox';

export const resubscribe = {
  params: Type.Object({
    unsubscribeId: Type.String({
      minLength: 1
    })
  }),
  response: {
    302: Type.Object({
      // this doesn't seem to work
      headers: Type.Object({
        location: Type.String()
      })
    }),
    500: Type.Object({
      error: Type.String()
    })
  }
};


Review comment (suggestion):
```suggestion
  })
```
This can all go because the serialization only works for response bodies, not headers.

Code after:
import { Type } from '@fastify/type-provider-typebox';

export const resubscribe = {
  params: Type.Object({
    unsubscribeId: Type.String({
      minLength: 1
    })
  })
};
Repository: freeCodeCamp/freeCodeCamp
File: api/src/schemas/email-subscription/unsubscribe.ts (TypeScript)

Code before:
import { Type } from '@fastify/type-provider-typebox';

export const unsubscribe = {
  params: Type.Object({
    unsubscribeId: Type.String({
      minLength: 1
    })
  }),
  response: {
    302: Type.Object({
      // this doesn't seem to work
      headers: Type.Object({
        location: Type.String()
      })
    }),
    500: Type.Object({
      error: Type.String()
    })
  }
};


Review comment (suggestion):
```suggestion
  })
```
Same as for resubscribe.

Code after:
import { Type } from '@fastify/type-provider-typebox';

export const unsubscribe = {
  params: Type.Object({
    unsubscribeId: Type.String({
      minLength: 1
    })
  })
};
Repository: freeCodeCamp/freeCodeCamp
File: api/src/schemas.ts (TypeScript)

Code before:
              answer: Type.String()
            })
          }),
          { minItems: 1 }
        )
      })
    }),
    response: {
      200: Type.Object({
        completedDate: Type.Number(),
        points: Type.Number(),
        alreadyCompleted: Type.Boolean(),
        examResults
      }),
      400: Type.Object({
        error: Type.String()
      }),
      403: Type.Object({
        error: Type.String()
      }),
      500: Type.Object({
        error: Type.String()
      })
    }
  },
  updateMyClassroomMode: {
    body: Type.Object({
      isClassroomAccount: Type.Boolean()
    }),
    response: {
      200: Type.Object({
        message: Type.Literal('flash.classroom-mode-updated'),
        type: Type.Literal('success')
      }),
      400: Type.Object({
        message: Type.Literal('flash.wrong-updating'),
        type: Type.Literal('danger')
      })
    }
  }
};


Review comment (suggestion):
I would be fine with it like this, but there's different areas in this file for the different types of endpoints, e.g. challenges, settings, etc. There's just comments in the file denoting this. This one is in the challenges area, it could be moved up to the settings area. 

Side note, we could probably separate this file into multiple files for the specific type of endpoints.

Code after:
      })
    }
  },
  examChallengeCompleted: {
    body: Type.Object({
      id: Type.String({ format: 'objectid', maxLength: 24, minLength: 24 }),
      challengeType: Type.Number(),
      userCompletedExam: Type.Object({
        examTimeInSeconds: Type.Number(),
        userExamQuestions: Type.Array(
          Type.Object({
            id: Type.String(),
            question: Type.String(),
            answer: Type.Object({
              id: Type.String(),
              answer: Type.String()
            })
          }),
          { minItems: 1 }
        )
      })
    }),
    response: {
      200: Type.Object({
        completedDate: Type.Number(),
        points: Type.Number(),
        alreadyCompleted: Type.Boolean(),
        examResults
      }),
      400: Type.Object({
        error: Type.String()
      }),
      403: Type.Object({
        error: Type.String()
      }),
      500: Type.Object({
        error: Type.String()
      })
    }
  }
};
Repository: freeCodeCamp/freeCodeCamp
File: e2e/delete-modal.spec.ts (TypeScript)

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

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

const supportEmail = 'support@freeCodeCamp.org';

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

test.describe('Delete Modal component', () => {
  test('renders the modal correctly', async ({ page }) => {
    await page.getByRole('button', { name: translations.buttons.menu }).click();
    await page
      .getByRole('link', { name: translations.buttons.settings })
      .click();
    await page
      .getByRole('button', { name: translations.settings.danger.delete })
      .click();

    await expect(
      page.getByRole('heading', {
        name: translations.settings.danger['delete-title']
      })
    ).toBeVisible();
    await expect(
      page.getByText(translations.settings.danger['delete-p1'])
    ).toBeVisible();
    await expect(
      page.getByText(translations.settings.danger['delete-p2'])
    ).toBeVisible();
    await expect(page.getByRole('link', { name: supportEmail })).toBeVisible();

    await expect(
      page.getByRole('button', { name: translations.settings.danger.nevermind })
    ).toBeVisible();
    await expect(
      page.getByRole('button', { name: translations.settings.danger.certain })
    ).toBeVisible();

Review comment (suggestion):
There should be a check for the appearance of the modal itself (an element with `role="dialog"`) before the checks for modal content. It's to ensure that we are displaying the content within a dialog, rather than a plain `div`.

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

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

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

test.describe('Delete Modal component', () => {
  test('should render the modal correctly', async ({ page }) => {
    await page
      .getByRole('button', { name: translations.settings.danger.delete })
      .click();

    // 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.
    const dialogs = await page.getByRole('dialog').all();
    expect(dialogs).toHaveLength(2);

    await expect(
      page.getByRole('heading', {
        name: translations.settings.danger['delete-title']
      })
    ).toBeVisible();
    await expect(
      page.getByText(translations.settings.danger['delete-p1'])
    ).toBeVisible();
    await expect(
      page.getByText(translations.settings.danger['delete-p2'])
    ).toBeVisible();

    const supportLink = page.getByRole('link', {
      name: 'support@freeCodeCamp.org'
    });
    await expect(supportLink).toBeVisible();
    await expect(supportLink).toHaveAttribute(
      'href',
      'mailto:support@freecodecamp.org'
Repository: freeCodeCamp/freeCodeCamp
File: e2e/delete-modal.spec.ts (TypeScript)

Code before:
      .getByRole('button', { name: translations.settings.danger.nevermind })
      .click();

    await expect(page).toHaveURL(/.*\/settings\/?#/);
    await expect(
      page.getByRole('heading', {
        name: translations.settings.danger['delete-title']
      })
    ).not.toBeVisible();
  });

  test('deletes account and redirects to /learn after confirmation', async ({
    page
  }) => {
    await page.getByRole('button', { name: translations.buttons.menu }).click();

    await page
      .getByRole('link', { name: translations.buttons.settings })
      .click();

    await page
      .getByRole('button', { name: translations.settings.danger.delete })
      .click();

    await page
      .getByRole('button', { name: translations.settings.danger.certain })
      .click();

    await expect(page).toHaveURL(/.*\/learn\/?/);
  });
});


Review comment (suggestion):
There should be a check for the appearance of the modal itself (an element with `role="dialog"`) before the checks for modal content. It's to ensure that we are displaying the content within a dialog, rather than a plain `div`.

Code after:
    const dialogs = await page.getByRole('dialog').all();
    expect(dialogs).toHaveLength(2);

    await page
      .getByRole('button', { name: translations.settings.danger.nevermind })
      .click();

    for (const dialog of dialogs) {
      await expect(dialog).not.toBeVisible();
    }
  });

  test('should close the modal and redirect to /learn after the user clicks delete', async ({
    page
  }) => {
    await page.route('*/**/account/delete', async route => {
      // intercept the endpoint to prevent user account from being deleted
      // as the deletion will cause subsequent tests to fail
      const json = {};
      await route.fulfill({ json });
    });

    await page
      .getByRole('button', { name: translations.settings.danger.delete })
      .click();

    const dialogs = await page.getByRole('dialog').all();
    expect(dialogs).toHaveLength(2);

    await page
      .getByRole('button', { name: translations.settings.danger.certain })
      .click();

    for (const dialog of dialogs) {
      await expect(dialog).not.toBeVisible();
    }

    await expect(page).toHaveURL(/.*\/learn\/?/);
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/formHelpers/form-validators.tsx (TypeScript)

Code before:
import React from 'react';
import { Trans } from 'react-i18next';

// Matches editor links for: Replit, Glitch, CodeSandbox, GitHub. NOT Codespaces, and NOT Gitpod yet
// Once safari allows negative lookbehinds, this can be used:
// |(?<!\.app)\.github\.dev
const editorRegex =
  /repl\.?it(\.com)?\/(@|join\/)|glitch\.com\/edit\/#!|codesandbox\.io\/s\/|github\.com/;
const fCCRegex =
  /codepen\.io\/freecodecamp|freecodecamp\.rocks|github\.com\/freecodecamp|\.freecodecamp\.org/i;
const localhostRegex = /localhost:|[::1]:/;
const httpRegex = /http(?!s|([^s]+?(localhost|[::1])))/;

function isPathRoot(urlString: string): boolean {
  try {
    return new URL(urlString).pathname !== '/';
  } catch {
    return false;
  }
}

type Validator = (value: string) => React.ReactElement | null;

export const editorValidator: Validator = value =>
  editorRegex.test(value) ? <Trans>validation.editor-url</Trans> : null;

export const fCCValidator: Validator = value =>
  fCCRegex.test(value) ? <Trans>validation.own-work-url</Trans> : null;

export const localhostValidator: Validator = value =>
  localhostRegex.test(value) ? (
    <Trans>validation.publicly-visible-url</Trans>
  ) : null;

export const httpValidator: Validator = value =>
  httpRegex.test(value) ? <Trans>validation.http-url</Trans> : null;


Review comment (suggestion):
This does not work for `http://127.0.0.1:3000`. We should use a library for this instead of coming up with the regex ourselves - there are too many valid formats.

Code after:
import React from 'react';
import { Trans } from 'react-i18next';
import { isPrivate } from './form-ip-utils';

// Matches editor links for: Replit, Glitch, CodeSandbox, GitHub. NOT Codespaces, and NOT Gitpod yet
// Once safari allows negative lookbehinds, this can be used:
// |(?<!\.app)\.github\.dev
const editorRegex =
  /repl\.?it(\.com)?\/(@|join\/)|glitch\.com\/edit\/#!|codesandbox\.io\/s\/|github\.com/;
const fCCRegex =
  /codepen\.io\/freecodecamp|freecodecamp\.rocks|github\.com\/freecodecamp|\.freecodecamp\.org/i;
const httpRegex = /^http:/;

function isPathRoot(urlString: string): boolean {
  try {
    return new URL(urlString).pathname !== '/';
  } catch {
    return false;
  }
}

type Validator = (value: string) => React.ReactElement | null;

export const editorValidator: Validator = value =>
  editorRegex.test(value) ? <Trans>validation.editor-url</Trans> : null;

export const fCCValidator: Validator = value =>
  fCCRegex.test(value) ? <Trans>validation.own-work-url</Trans> : null;

export const localhostValidator: Validator = value =>
  isPrivate(value) ? <Trans>validation.publicly-visible-url</Trans> : null;

export const httpValidator: Validator = value =>
  !isPrivate(value) && httpRegex.test(value) ? (
    <Trans>validation.http-url</Trans>
  ) : null;
Repository: freeCodeCamp/freeCodeCamp
File: api/src/routes/user.ts (TypeScript)

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

        const completedSurveysP = fastify.prisma.survey.findMany({
          where: { userId: req.session.user.id }
        });

        const msUsernameP = fastify.prisma.msUsername.findFirst({
          where: { userId: req.session.user.id }
        });

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

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

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

        const {
          username,
          usernameDisplay,
          completedChallenges,
          progressTimestamps,
          twitter,

Review comment (suggestion):
After Oliver's PR, I believe this is what we now want:

```suggestion
          where: { userId: req.user?.id }
```

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

        const completedSurveysP = fastify.prisma.survey.findMany({
          where: { userId: req.session.user.id }
        });

        const msUsernameP = fastify.prisma.msUsername.findFirst({
          where: { userId: req.user?.id }
        });

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

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

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

        const {
          username,
          usernameDisplay,
          completedChallenges,
          progressTimestamps,
          twitter,
Repository: freeCodeCamp/freeCodeCamp
File: tools/ui-components/src/drop-down/drop-down.test.tsx (TypeScript)

Code before:

    const dropDownTrigger = screen.getByRole('button', { name: 'test' });
    await userEvent.click(dropDownTrigger);
    const unorderedList = screen.getByRole('menu');
    const Item = within(unorderedList).getByRole('menuitem');

    expect(Item).toBeInTheDocument();
    expect(Item).toHaveAttribute('href', 'https://www.freecodecamp.org');
  });

  it('should render a button element if the `href` and `disabled` props are both defined', async () => {
    render(
      <Dropdown>
        <Dropdown.Toggle>test</Dropdown.Toggle>
        <Dropdown.Menu>
          <MenuItem href='https://www.freecodecamp.org' disabled>
            freeCodeCamp
          </MenuItem>
        </Dropdown.Menu>
      </Dropdown>
    );

    const dropDownTrigger = screen.getByRole('button', { name: 'test' });
    await userEvent.click(dropDownTrigger);
    const unorderedList = screen.getByRole('menu');
    const Item = within(unorderedList).getByRole('menuitem');

    expect(Item).toBeInTheDocument();
    expect(Item).toHaveAttribute('aria-disabled', 'true');
  });
});


Review comment (suggestion):
```suggestion
    const Item = within(unorderedList).getByRole('menuitem', { name: 'freeCodeCamp' });
```

Code after:
        <Dropdown.Toggle>test</Dropdown.Toggle>
        <Dropdown.Menu>
          <MenuItem href='https://www.freecodecamp.org'>freeCodeCamp</MenuItem>
        </Dropdown.Menu>
      </Dropdown>
    );

    const dropDownTrigger = screen.getByRole('button', { name: 'test' });
    await userEvent.click(dropDownTrigger);
    const unorderedList = screen.getByRole('menu');
    const item = within(unorderedList).getByRole('menuitem', {
      name: 'freeCodeCamp'
    });

    expect(item).toBeInTheDocument();
    expect(item).toHaveAttribute('href', 'https://www.freecodecamp.org');
  });

  it('should render a button element if the `href` and `disabled` props are both defined', async () => {
    render(
      <Dropdown>
        <Dropdown.Toggle>test</Dropdown.Toggle>
        <Dropdown.Menu>
          <MenuItem href='https://www.freecodecamp.org' disabled>
            freeCodeCamp
          </MenuItem>
        </Dropdown.Menu>
      </Dropdown>
    );

    const dropDownTrigger = screen.getByRole('button', { name: 'test' });
    await userEvent.click(dropDownTrigger);
    const unorderedList = screen.getByRole('menu');
    const item = within(unorderedList).getByRole('menuitem', {
      name: 'freeCodeCamp'
    });

    expect(item).toBeInTheDocument();
    expect(item).toHaveAttribute('aria-disabled', 'true');
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/Header/components/login.tsx (TypeScript)

Code before:
  children?: ReactNode;
  'data-test-label'?: string;
  isSignedIn?: boolean;
}

const Login = ({
  block,
  children,
  'data-test-label': dataTestLabel,
  isSignedIn
}: LoginProps): JSX.Element => {
  const { t } = useTranslation();

  const href = isSignedIn ? `${homeLocation}/learn` : `${apiLocation}/signin`;
  return (
    <a
      className={(block ? 'btn-cta-big btn-block' : '') + ' signup-btn btn-cta'}
      data-test-label={dataTestLabel}
      data-playwright-test-label='header-sign-in-button'
      href={href}
    >
      <span className='login-btn-icon'>
        <FontAwesomeIcon icon={faRightToBracket} />
        <span className='sr-only'> {t('buttons.sign-in')}</span>
      </span>
      <span className='login-btn-text'>{children || t('buttons.sign-in')}</span>
    </a>
  );
};

Login.displayName = 'Login';

export default connect(mapStateToProps)(Login);


Review comment (bug):
I just want to point out this can fail, and you will not necessarily know. The `onclick` fires before navigating to the `href`.

So, provided that is the behaviour (analytics) you want, there is no issue.

Code after:
  block?: boolean;
  children?: ReactNode;
  'data-test-label'?: string;
  isSignedIn?: boolean;
}

const Login = ({
  block,
  children,
  'data-test-label': dataTestLabel,
  isSignedIn
}: LoginProps): JSX.Element => {
  const { t } = useTranslation();

  const href = isSignedIn ? `${homeLocation}/learn` : `${apiLocation}/signin`;
  return (
    <a
      className={(block ? 'btn-cta-big btn-block' : '') + ' signup-btn btn-cta'}
      data-test-label={dataTestLabel}
      data-playwright-test-label='header-sign-in-button'
      href={href}
      onClick={() => {
        callGA({
          event: 'sign_in'
        });
      }}
    >
      <span className='login-btn-icon'>
        <FontAwesomeIcon icon={faRightToBracket} />
        <span className='sr-only'> {t('buttons.sign-in')}</span>
      </span>
      <span className='login-btn-text'>{children || t('buttons.sign-in')}</span>
    </a>
  );
};

Login.displayName = 'Login';

export default connect(mapStateToProps)(Login);