ronantakizawa/codereview-bench huggingface dataset:
Repository: freeCodeCamp/freeCodeCamp
File: client/src/redux/donation-saga.js (JavaScript)

Code before:
export function* setDonationCookie() {
  if (document?.cookie) {
    const isDonating = yield select(isDonatingSelector);
    const isDonorCookieSet = document.cookie
      .split(';')
      .some(item => item.trim().startsWith('isDonor=true'));
    if (isDonating) {
      if (!isDonorCookieSet) {
        document.cookie = 'isDonor=true';
      }
    }
  }
}

export function* updateCardSaga() {
  yield put(updateCardRedirecting());
  try {
    const {
      data: { session_id }
    } = yield call(updateStripeCard);

    if (!session_id) throw new Error('No session_id');
    const stripe = yield call(loadStripe, stripePublicKey);
    stripe.redirectToCheckout({
      sessionId: session_id
    });
  } catch (error) {
    console.log('error');
    yield put(updateCardError(updateCardErrorMessage));
  }
}

export function createDonationSaga(types) {
  return [
    takeEvery(types.tryToShowDonationModal, showDonateModalSaga),
    takeLeading(types.postCharge, postChargeSaga),
    takeEvery(types.fetchUserComplete, setDonationCookie),
    takeLeading(types.updateCard, updateCardSaga)
  ];
}


Review comment (suggestion):
```suggestion
    stripe.redirectToCheckout({ sessionId });
```

Code after:
export function* setDonationCookie() {
  if (document?.cookie) {
    const isDonating = yield select(isDonatingSelector);
    const isDonorCookieSet = document.cookie
      .split(';')
      .some(item => item.trim().startsWith('isDonor=true'));
    if (isDonating) {
      if (!isDonorCookieSet) {
        document.cookie = 'isDonor=true';
      }
    }
  }
}

export function* updateCardSaga() {
  yield put(updateCardRedirecting());
  try {
    const {
      data: { sessionId }
    } = yield call(updateStripeCard);

    if (!sessionId) throw new Error('No sessionId');
    const stripe = yield call(loadStripe, stripePublicKey);
    stripe.redirectToCheckout({ sessionId });
  } catch (error) {
    yield put(updateCardError(updateCardErrorMessage));
  }
}

export function createDonationSaga(types) {
  return [
    takeEvery(types.tryToShowDonationModal, showDonateModalSaga),
    takeLeading(types.postCharge, postChargeSaga),
    takeEvery(types.fetchUserComplete, setDonationCookie),
    takeLeading(types.updateCard, updateCardSaga)
  ];
}
Repository: freeCodeCamp/freeCodeCamp
File: client/src/redux/donation-saga.test.js (JavaScript)

Code before:
      .not.call.fn(addDonation)
      .put(postChargeComplete())
      .call(setDonationCookie)
      .put(executeGA(paypalAnalyticsDataMock))
      .run();
  });

  it('does not call api for Patreon', () => {
    const patreonDataMock = {
      payload: { ...postChargeDataMock.payload, paymentProvider: 'patreon' }
    };

    const patreonAnalyticsDataMock = analyticsDataMock;
    patreonAnalyticsDataMock.action = 'Donate Page Patreon Payment Redirection';
    patreonAnalyticsDataMock.event = 'donation_related';
    return expectSaga(postChargeSaga, patreonDataMock)
      .not.call.fn(addDonation)
      .not.call.fn(postChargeStripeCard)
      .not.call.fn(postChargeStripe)
      .put(executeGA(patreonAnalyticsDataMock))
      .run();
  });

  it('handles successful card update', () => {
    updateStripeCard.mockResolvedValue({
      data: { session_id: 'expected data' }
    });

    return expectSaga(updateCardSaga)
      .put(updateCardRedirecting())
      .call(updateStripeCard)
      .not.put(updateCardError())
      .run();
  });

  it('handles errors correctly for card update', () => {
    updateStripeCard.mockResolvedValue({
      data: 'unexpected data'
    });

    return expectSaga(updateCardSaga)
      .put(updateCardRedirecting())
      .call(updateStripeCard)
      .put(updateCardError())
      .run();
  });
});


Review comment (suggestion):
```suggestion
      data: { sessionId: 'expected data' }
```

Code after:
      .not.call.fn(addDonation)
      .put(postChargeComplete())
      .call(setDonationCookie)
      .put(executeGA(paypalAnalyticsDataMock))
      .run();
  });

  it('does not call api for Patreon', () => {
    const patreonDataMock = {
      payload: { ...postChargeDataMock.payload, paymentProvider: 'patreon' }
    };

    const patreonAnalyticsDataMock = analyticsDataMock;
    patreonAnalyticsDataMock.action = 'Donate Page Patreon Payment Redirection';
    patreonAnalyticsDataMock.event = 'donation_related';
    return expectSaga(postChargeSaga, patreonDataMock)
      .not.call.fn(addDonation)
      .not.call.fn(postChargeStripeCard)
      .not.call.fn(postChargeStripe)
      .put(executeGA(patreonAnalyticsDataMock))
      .run();
  });

  it('handles successful card update', () => {
    updateStripeCard.mockResolvedValue({
      data: { sessionId: 'expected data' }
    });

    return expectSaga(updateCardSaga)
      .put(updateCardRedirecting())
      .call(updateStripeCard)
      .not.put(updateCardError())
      .run();
  });

  it('handles errors correctly for card update', () => {
    updateStripeCard.mockResolvedValue({
      data: 'unexpected data'
    });

    return expectSaga(updateCardSaga)
      .put(updateCardRedirecting())
      .call(updateStripeCard)
      .put(updateCardError())
      .run();
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: client/src/utils/ajax.ts (TypeScript)

Code before:

export function getGenerateExam(
  challengeId: string
): Promise<GenerateExamResponseWithData> {
  return get(`/exam/${challengeId}`);
}

/** POST **/

interface Donation {
  email: string;
  amount: number;
  duration: string;
  provider: string;
  subscriptionId: string;
  customerId: string;
  startDate: Date;
}
// TODO: Verify if the body has and needs this Donation type. The api seems to
// just need the body to exist, but doesn't seem to use the properties.
export function addDonation(body: Donation): Promise<ResponseWithData<void>> {
  return post('/donate/add-donation', body);
}

export function updateStripeCard() {
  return post('/donate/update-stripe-card');
}

export function postChargeStripe(
  body: Donation
): Promise<ResponseWithData<void>> {
  return post('/donate/charge-stripe', body);
}

export function postChargeStripeCard(
  body: Donation
): Promise<ResponseWithData<void>> {
  return post('/donate/charge-stripe-card', body);
}
interface Report {
  username: string;
  reportDescription: string;
}
export function postReportUser(body: Report): Promise<ResponseWithData<void>> {
  return post('/user/report-user', body);
}

// Both are called without a payload in danger-zone-saga,
// which suggests both are sent without any body
// TODO: Convert to DELETE
export function postDeleteAccount(): Promise<ResponseWithData<void>> {

Review comment (suggestion):
```suggestion
  return put('/donate/update-stripe-card');
```
Nitpicky, I know, but `put` makes sense here, since we're updating an existing resource.


Code after:

export function getGenerateExam(
  challengeId: string
): Promise<GenerateExamResponseWithData> {
  return get(`/exam/${challengeId}`);
}

/** POST **/

interface Donation {
  email: string;
  amount: number;
  duration: string;
  provider: string;
  subscriptionId: string;
  customerId: string;
  startDate: Date;
}
// TODO: Verify if the body has and needs this Donation type. The api seems to
// just need the body to exist, but doesn't seem to use the properties.
export function addDonation(body: Donation): Promise<ResponseWithData<void>> {
  return post('/donate/add-donation', body);
}

export function updateStripeCard() {
  return put('/donate/update-stripe-card');
}

export function postChargeStripe(
  body: Donation
): Promise<ResponseWithData<void>> {
  return post('/donate/charge-stripe', body);
}

export function postChargeStripeCard(
  body: Donation
): Promise<ResponseWithData<void>> {
  return post('/donate/charge-stripe-card', body);
}
interface Report {
  username: string;
  reportDescription: string;
}
export function postReportUser(body: Report): Promise<ResponseWithData<void>> {
  return post('/user/report-user', body);
}

// Both are called without a payload in danger-zone-saga,
// which suggests both are sent without any body
// TODO: Convert to DELETE
export function postDeleteAccount(): Promise<ResponseWithData<void>> {
Repository: freeCodeCamp/freeCodeCamp
File: api-server/src/server/utils/donation.js (JavaScript)

Code before:
  };
  await createAsyncUserDonation(user, donation);
  return res.status(200).json({ isDonating: true });
}

export async function handleStripeCardUpdateSession(req, app, stripe) {
  const {
    user: { id }
  } = req;

  const { Donation } = app.models;
  log('Updating stripe card for user: ', ObjectId(id));

  /* eslint-disable camelcase */
  try {
    // multiple donations support should be added
    const donation = await Donation.findOne({
      where: { userId: ObjectId(id) }
    });

    if (!donation) throw Error();

    const { customerId, subscriptionId } = donation;

    // updating customer payment method is handled by webhook handler
    const session = await stripe.checkout.sessions.create({
      payment_method_types: ['card'],
      mode: 'setup',
      customer: customerId,
      setup_intent_data: {
        metadata: {
          customer_id: customerId,
          subscription_id: subscriptionId
        }
      },
      success_url: `${process.env.HOME_LOCATION}/update-stripe-card?session_id={CHECKOUT_SESSION_ID}`,
      cancel_url: `${process.env.HOME_LOCATION}/update-stripe-card`
    });
    // /* eslint-enable camelcase */
    return { session_id: session.id };
  } catch (err) {
    throw Error(err);
  }
}


Review comment (suggestion):
Sorry for the stupid question, but does this return the same `subscription` id as we send it? I just want to be 100% sure that updating a subscription doesn't change the id!

Code after:
  await createAsyncUserDonation(user, donation);
  return res.status(200).json({ isDonating: true });
}

export async function handleStripeCardUpdateSession(req, app, stripe) {
  const {
    user: { id }
  } = req;

  const { Donation } = app.models;
  log('Updating stripe card for user: ', id);

  // multiple donations support should be added
  const donation = await Donation.findOne({
    where: { userId: id, provider: 'stripe' }
  });

  if (!donation) throw Error('Stripe donation record not found');

  const { customerId, subscriptionId } = donation;

  log(subscriptionId);

  // Create a Stripe checkout session
  // updating customer payment method is handled by webhook handler
  const session = await stripe.checkout.sessions.create({
    payment_method_types: ['card'],
    mode: 'setup',
    customer: customerId,
    setup_intent_data: {
      metadata: {
        customer_id: customerId,
        subscription_id: subscriptionId
      }
    },
    success_url: `${process.env.HOME_LOCATION}/update-stripe-card?session_id={CHECKOUT_SESSION_ID}`,
    cancel_url: `${process.env.HOME_LOCATION}/update-stripe-card`
  });
  return { sessionId: session.id };
}
Repository: freeCodeCamp/freeCodeCamp
File: e2e/update-stripe-card-default.spec.ts (TypeScript)

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

let page: Page;
test.beforeAll(async ({ browser }) => {
  page = await browser.newPage();
  await page.goto('/update-stripe-card');
});

test.describe('Update Card Page for Non-Donor Authenticated User', () => {
  test.use({ storageState: 'playwright/.auth/certified-user.json' });
  test('should display the correct', async () => {
    await expect(page).toHaveTitle(
      `${translations.misc['update-your-card']} | freeCodeCamp.org`
    );
  });
  test('should display the info for signedin non-donor', async () => {
    const h1 = page.locator('h1');
    await expect(h1).toHaveText(
      `${translations.learn['donation-record-not-found']}`
    );
  });
});

// Unauthrorized, and donor user states should be added here in upcoming PRs


Review comment (suggestion):
```suggestion
```
Sharing pages between tests is a bit of an anti-pattern. Instead it's fine to simply write a larger test.

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

test.describe('Update Card Page for Non-Donor Authenticated User', () => {
  test.use({ storageState: 'playwright/.auth/certified-user.json' });
  test('should render correctly', async ({ page }) => {
    await page.goto('/update-stripe-card');
    await expect(page).toHaveTitle(
      `${translations.misc['update-your-card']} | freeCodeCamp.org`
    );
    const h1 = page.locator('h1');
    await expect(h1).toHaveText(
      `${translations.learn['donation-record-not-found']}`
    );
  });
});

// Unauthrorized, and donor user states should be added here in upcoming PRs
Repository: freeCodeCamp/freeCodeCamp
File: e2e/update-stripe-card-default.spec.ts (TypeScript)

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

let page: Page;
test.beforeAll(async ({ browser }) => {
  page = await browser.newPage();
  await page.goto('/update-stripe-card');
});

test.describe('Update Card Page for Non-Donor Authenticated User', () => {
  test.use({ storageState: 'playwright/.auth/certified-user.json' });
  test('should display the correct', async () => {
    await expect(page).toHaveTitle(
      `${translations.misc['update-your-card']} | freeCodeCamp.org`
    );
  });
  test('should display the info for signedin non-donor', async () => {
    const h1 = page.locator('h1');
    await expect(h1).toHaveText(
      `${translations.learn['donation-record-not-found']}`
    );
  });
});

// Unauthrorized, and donor user states should be added here in upcoming PRs


Review comment (suggestion):
```suggestion
import { test, expect } from '@playwright/test';
```

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

test.describe('Update Card Page for Non-Donor Authenticated User', () => {
  test.use({ storageState: 'playwright/.auth/certified-user.json' });
  test('should render correctly', async ({ page }) => {
    await page.goto('/update-stripe-card');
    await expect(page).toHaveTitle(
      `${translations.misc['update-your-card']} | freeCodeCamp.org`
    );
    const h1 = page.locator('h1');
    await expect(h1).toHaveText(
      `${translations.learn['donation-record-not-found']}`
    );
  });
});

// Unauthrorized, and donor user states should be added here in upcoming PRs
Repository: freeCodeCamp/freeCodeCamp
File: e2e/update-stripe-card-default.spec.ts (TypeScript)

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

let page: Page;
test.beforeAll(async ({ browser }) => {
  page = await browser.newPage();
  await page.goto('/update-stripe-card');
});

test.describe('Update Card Page for Non-Donor Authenticated User', () => {
  test.use({ storageState: 'playwright/.auth/certified-user.json' });
  test('should display the correct', async () => {
    await expect(page).toHaveTitle(
      `${translations.misc['update-your-card']} | freeCodeCamp.org`
    );
  });
  test('should display the info for signedin non-donor', async () => {
    const h1 = page.locator('h1');
    await expect(h1).toHaveText(
      `${translations.learn['donation-record-not-found']}`
    );
  });
});

// Unauthrorized, and donor user states should be added here in upcoming PRs


Review comment (suggestion):
```suggestion
  test('should render correctly', async ({ page }) => {
    await expect(page).toHaveTitle(
      `${translations.misc['update-your-card']} | freeCodeCamp.org`
    );
    const h1 = page.locator('h1');
    await expect(h1).toHaveText(
      `${translations.learn['donation-record-not-found']}`
    );
  });
```

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

test.describe('Update Card Page for Non-Donor Authenticated User', () => {
  test.use({ storageState: 'playwright/.auth/certified-user.json' });
  test('should render correctly', async ({ page }) => {
    await page.goto('/update-stripe-card');
    await expect(page).toHaveTitle(
      `${translations.misc['update-your-card']} | freeCodeCamp.org`
    );
    const h1 = page.locator('h1');
    await expect(h1).toHaveText(
      `${translations.learn['donation-record-not-found']}`
    );
  });
});

// Unauthrorized, and donor user states should be added here in upcoming PRs
Repository: freeCodeCamp/freeCodeCamp
File: e2e/challenge-description.spec.ts (TypeScript)

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

test.beforeEach(async ({ page }) => {
  await page.goto(
    '/learn/foundational-c-sharp-with-microsoft/write-your-first-code-using-c-sharp/trophy-write-your-first-code-using-c-sharp'
  );
});

test.describe('Challenge Description Component Tests', () => {
  test('check components rendering', async ({ page }) => {
    const challengeDescription = page.getByTestId('challenge-description');
    await expect(challengeDescription).toBeVisible();
    await expect(challengeDescription).toHaveText(/ */);

    const link = page.getByRole('link', { name: 'your achievements page' });
    await expect(link).toHaveAttribute(
      'href',
      'https://learn.microsoft.com/users/me/achievements#trophies-section'
    );
    await expect(link).toHaveAttribute('target', '_blank');
    await expect(link).toHaveAttribute('rel', 'noreferrer');
  });
});


Review comment (suggestion):
```suggestion
  test('should display the content correctly', async ({ page }) => {
```

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

test.beforeEach(async ({ page }) => {
  await page.goto(
    '/learn/foundational-c-sharp-with-microsoft/write-your-first-code-using-c-sharp/trophy-write-your-first-code-using-c-sharp'
  );
});

test.describe('Challenge Description Component Tests', () => {
  test('should display the content correctly', async ({ page }) => {
    const challengeDescription = page.getByTestId('challenge-description');
    await expect(challengeDescription).toBeVisible();
    await expect(challengeDescription).toHaveText(/ */);

    const link = page.getByRole('link', { name: 'your achievements page' });
    await expect(link).toHaveAttribute(
      'href',
      'https://learn.microsoft.com/users/me/achievements#trophies-section'
    );
    await expect(link).toHaveAttribute('target', '_blank');
    await expect(link).toHaveAttribute('rel', 'noreferrer');
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: client/src/templates/Challenges/classic/editor.tsx (TypeScript)

Code before:
      updateDescriptionZone();
      updateOutputZone();
    }
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [props.dimensions]);

  function updateDescriptionZone() {
    const editor = dataRef.current.editor;
    editor?.changeViewZones(changeAccessor => {
      changeAccessor.removeZone(dataRef.current.descriptionZoneId);
      descriptionZoneCallback(changeAccessor);
    });
  }

  const { theme } = props;

  const preferDarkScheme = window.matchMedia(
    '(prefers-color-scheme: dark)'
  ).matches;
  const editorSystemTheme = preferDarkScheme ? 'vs-dark-custom' : 'vs-custom';
  const editorTheme =
    theme === Themes.Night
      ? 'vs-dark-custom'
      : theme === Themes.Default
      ? 'vs-custom'
      : editorSystemTheme;
  return (
    <Suspense fallback={<Loader loaderDelay={600} />}>
      <span className='notranslate'>
        <MonacoEditor
          editorDidMount={editorDidMount}
          editorWillMount={editorWillMount}
          onChange={onChange}
          options={{ ...options, folding: !hasEditableRegion() }}
          theme={editorTheme}
        />
      </span>
    </Suspense>
  );
};

Editor.displayName = 'Editor';

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


Review comment (suggestion):
Apparently prettier thinks these should all line up. I'm actually a little surprised we have nested ternaries here. But if we are going to keep them, then I think it makes sense to nest the second one. I'll put it back.

Code after:
      updateDescriptionZone();
      updateOutputZone();
    }
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [props.dimensions]);

  function updateDescriptionZone() {
    const editor = dataRef.current.editor;
    editor?.changeViewZones(changeAccessor => {
      changeAccessor.removeZone(dataRef.current.descriptionZoneId);
      descriptionZoneCallback(changeAccessor);
    });
  }

  const { theme } = props;

  const preferDarkScheme = window.matchMedia(
    '(prefers-color-scheme: dark)'
  ).matches;
  const editorSystemTheme = preferDarkScheme ? 'vs-dark-custom' : 'vs-custom';
  const editorTheme =
    theme === Themes.Night
      ? 'vs-dark-custom'
      : theme === Themes.Default
        ? 'vs-custom'
        : editorSystemTheme;
  return (
    <Suspense fallback={<Loader loaderDelay={600} />}>
      <span className='notranslate'>
        <MonacoEditor
          editorDidMount={editorDidMount}
          editorWillMount={editorWillMount}
          onChange={onChange}
          options={{ ...options, folding: !hasEditableRegion() }}
          theme={editorTheme}
        />
      </span>
    </Suspense>
  );
};

Editor.displayName = 'Editor';

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