Skip to content

ENT-11429: Add billing address capture and fix tracking event issue#157

Open
gshivajibiradar wants to merge 6 commits intomainfrom
ENT-11429-billing-address
Open

ENT-11429: Add billing address capture and fix tracking event issue#157
gshivajibiradar wants to merge 6 commits intomainfrom
ENT-11429-billing-address

Conversation

@gshivajibiradar
Copy link
Copy Markdown
Contributor

@gshivajibiradar gshivajibiradar commented Mar 17, 2026

Title
ENT-11429: Add billing address capture for checkout and fix subscribe-button tracking event

ticket
https://2u-internal.atlassian.net/browse/ENT-11429

Summary
Adds billing address validation fields to checkout billing schema (full name, country, address line1/line2, city, state, zip).
Fixes subscribe-button tracking flow so billing-details click tracking is emitted correctly.
Updates billing-details tests to cover tracking behavior on subscribe click.
Keeps only implementation code changes (no checklist/guide markdown docs included).

What Changed
Updated checkout billing schema and validation rules.
Updated subscribe button behavior and click handling.
Updated billing-details page submit/click handling.
Updated billing-details test assertions for tracking event.

Why
Ensure Essentials checkout collects required billing address data consistently.
Prevent missed analytics events on subscribe button click.
Keep behavior aligned with expected Stripe billing flow and existing checkout UX.

Testing
npm run lint -- --fix
npm test -- --no-coverage
Result: 48 test suites passed, 363 tests passed.
Risk / Impact
Low to medium.
Changes are localized to billing-details flow and related tests.
No API contract changes.
Screenshot 2026-03-26 104116
Screenshot 2026-03-26 104228
Screenshot 2026-03-26 104605
Screenshot 2026-03-26 104456

Checklist
Code builds locally
Lint passes
Tests pass
No unwanted docs/files included in commit

Copilot AI review requested due to automatic review settings March 17, 2026 08:40
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 17, 2026

Codecov Report

❌ Patch coverage is 80.89888% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.83%. Comparing base (85345c2) to head (f38a527).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...ponents/StatefulButton/StatefulSubscribeButton.tsx 51.72% 14 Missing ⚠️
src/components/ErrorPage/ErrorPage.tsx 77.77% 2 Missing ⚠️
...nents/billing-details-pages/BillingDetailsPage.tsx 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #157      +/-   ##
==========================================
+ Coverage   84.41%   84.83%   +0.42%     
==========================================
  Files         150      150              
  Lines        2579     2684     +105     
  Branches      504      545      +41     
==========================================
+ Hits         2177     2277     +100     
- Misses        378      384       +6     
+ Partials       24       23       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the self-service checkout Billing Details step to (1) add billing address fields to the billing Zod schema and (2) fix analytics so the “Subscribe” click tracking event is emitted correctly, with corresponding test updates.

Changes:

  • Expanded BillingDetailsSchema to validate billing name + address fields.
  • Refactored Billing Details “Subscribe” click handling to emit tracking on click.
  • Updated Billing Details page tests to assert the subscribe click tracking event.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File Description
src/constants/checkout.ts Adds full billing address fields + validation rules to BillingDetailsSchema.
src/components/billing-details-pages/BillingDetailsPage.tsx Adds click-tracking flow and updates form defaults/submit handling for Billing Details.
src/components/StatefulButton/StatefulSubscribeButton.tsx Adds optional parent onClick hook and invokes it before the Stripe confirm flow.
src/components/billing-details-pages/tests/BillingDetailsPage.test.tsx Updates test to assert subscribe click tracking event behavior.
Comments suppressed due to low confidence (1)

src/components/StatefulButton/StatefulSubscribeButton.tsx:112

  • onClickHandler invokes the parent onClick (which triggers handleSubmit) but doesn’t await it before proceeding to set pending state and call Stripe confirm(). Since the subscribe button is also type: 'submit', confirm can start even if RHF validation fails and before any setFormData completes. If the parent handler is meant to validate/persist billing data prior to confirming payment, make the prop async and await it (and/or prevent default submit and sequence validation explicitly).
  const onClickHandler = async () => {
    // Call the parent's onClick handler first (e.g., for tracking)
    onClick?.();

    // Sets the button to pending state and then calls confirm()
    setStatefulButtonState('pending');

    // Calls confirm() to start the Stripe checkout flow.
    let response;
    try {
      if (checkoutIntent) {
        const { uuid, country, state } = checkoutIntent;
        const tncCheckoutUpdateRequest: CheckoutIntentPatchRequestSchema = {
          country,
          state,
          termsMetadata: termsAndConditions,
        };
        await patchCheckoutIntent({
          uuid,
          requestData: tncCheckoutUpdateRequest,
        });
      }
      response = await confirm({
        redirect: 'if_required',
        returnUrl: `${window.location.href}/${CheckoutSubstepKey.Success}`,
      });
    } catch (error) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/billing-details-pages/BillingDetailsPage.tsx
Comment thread src/components/billing-details-pages/BillingDetailsPage.tsx Outdated
Comment on lines 83 to 85
@@ -79,6 +84,9 @@ const StatefulSubscribeButton = () => {
const isFormValid = canConfirm && !hasInvalidTerms;

Comment thread src/components/StatefulButton/StatefulSubscribeButton.tsx
Comment thread src/components/StatefulButton/StatefulSubscribeButton.tsx
Comment thread src/constants/checkout.ts
Comment thread src/components/billing-details-pages/BillingDetailsPage.tsx
Copilot AI review requested due to automatic review settings March 18, 2026 04:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the enterprise checkout Billing Details step to (1) capture/validate billing address details and (2) fix analytics so the “Subscribe” click tracking event is emitted reliably before any form validation/submit flow.

Changes:

  • Extended the billing details Zod schema to include billing address fields (name, country, address lines, city, state, zip).
  • Refactored Billing Details page + Stripe AddressElement integration to persist address fields into the form/store and to emit subscribe-click tracking.
  • Updated Billing Details + subscribe button tests to seed/address required fields and assert tracking behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/constants/checkout.ts Adds billing address fields to BillingDetailsSchema validation.
src/components/billing-details-pages/BillingDetailsPage.tsx Adjusts RHF defaults and moves subscribe tracking/submit orchestration.
src/components/Stepper/StepperContent/BillingDetailsContent.tsx Passes form down to billing form fields.
src/components/FormFields/BillingFormFields.tsx Wires Stripe AddressElement changes into RHF + Zustand store.
src/components/StatefulButton/StatefulSubscribeButton.tsx Adds optional parent onClick hook and updates required-field gating.
src/components/billing-details-pages/tests/BillingDetailsPage.test.tsx Seeds required billing fields and asserts subscribe tracking event.
src/components/StatefulButton/tests/StatefulSubscribeButton.test.tsx Updates test fixtures to include new required billing fields.
Comments suppressed due to low confidence (3)

src/constants/checkout.ts:308

  • BillingDetailsSchema does not include confirmRecurringSubscription, but the UI (TermsAndConditionsCheckboxes + StatefulSubscribeButton) treats it as a required field. Because the form uses zodResolver, unknown fields are stripped from the submitted payload; when setFormData replaces the step state, confirmRecurringSubscription can be dropped, causing the checkbox to reset and the subscribe button to become invalid. Add confirmRecurringSubscription to the schema (with the same refine(value => value) pattern as the other confirmations) or make the schema passthrough() if extra fields must be preserved.
export const EssentialsPageDetails = {
  AcademicSelection: {
    step: 'AcademicSelection',
    substep: undefined,
    formSchema: AcademicSelectionSchema,
    route: EssentialsPageRoute.AcademicSelection,
    title: defineMessages({
      id: 'essentials.academicSelection.title',
      defaultMessage: 'Academic Selection',

src/components/StatefulButton/StatefulSubscribeButton.tsx:105

  • onClickHandler calls the parent onClick but doesn’t await it. In this PR the parent onClick triggers a handleSubmit call; since it’s not awaited, Stripe confirm() can proceed even if validation/persist fails. Make the prop support async (onClick?: () => void | Promise<void>) and await it (and ideally allow it to signal “do not proceed”) before setting the button to pending / calling confirm().
    // Call the parent's onClick handler first (e.g., for tracking)
    onClick?.();

    // Sets the button to pending state and then calls confirm()
    setStatefulButtonState('pending');

    // Calls confirm() to start the Stripe checkout flow.
    let response;
    try {

src/components/StatefulButton/StatefulSubscribeButton.tsx:197

  • This component is written in TypeScript and already has a typed props interface; adding propTypes/defaultProps here is redundant and inconsistent with the rest of the TSX components in this repo (this is the only .propTypes usage under src/components). Prefer relying on TypeScript (e.g., default the destructured prop value) and remove the prop-types usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/billing-details-pages/BillingDetailsPage.tsx
Comment thread src/components/billing-details-pages/BillingDetailsPage.tsx
@gshivajibiradar gshivajibiradar force-pushed the ENT-11429-billing-address branch from 029fb31 to ff956d5 Compare March 18, 2026 05:00
Copilot AI review requested due to automatic review settings March 18, 2026 05:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds billing-address capture to the Billing Details checkout step (via Stripe AddressElement + schema validation) and adjusts the subscribe click flow so the billing-details subscribe tracking event fires reliably, with tests updated accordingly.

Changes:

  • Expanded BillingDetailsSchema to validate billing address fields (name, country, address lines, city/state/zip).
  • Wired Stripe AddressElement changes into react-hook-form + the checkout form store.
  • Updated subscribe click handling and tests to assert the tracking event on subscribe click.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/constants/checkout.ts Adds billing address fields to the Zod billing details schema.
src/components/billing-details-pages/BillingDetailsPage.tsx Initializes default billing address values and moves subscribe tracking to the button click handler.
src/components/Stepper/StepperContent/BillingDetailsContent.tsx Passes the RHF form instance into BillingFormFields.
src/components/FormFields/BillingFormFields.tsx Implements Stripe AddressElement change mapping into form/store and renders fullName validation feedback.
src/components/FormFields/tests/BillingFormFields.test.tsx Adds tests for Stripe element rendering, store mapping on change, and validation feedback rendering.
src/components/StatefulButton/StatefulSubscribeButton.tsx Adds an optional onClick hook and refines required-field validity logic.
src/components/StatefulButton/tests/StatefulSubscribeButton.test.tsx Updates mocked billing details store data to include new required billing address fields.
src/components/billing-details-pages/tests/BillingDetailsPage.test.tsx Seeds billing address data and asserts the subscribe click tracking event.
Comments suppressed due to low confidence (1)

src/constants/checkout.ts:307

  • BillingDetailsSchema is missing validation for confirmRecurringSubscription, but the Billing Details UI (TermsAndConditionsCheckboxes) and subscribe-button validity checks rely on this field. This makes schema-derived BillingDetailsData inconsistent with actual form/store shape and prevents react-hook-form/zod from ever producing an error for that checkbox. Add confirmRecurringSubscription: z.boolean().refine(...) to this schema (with an appropriate message) so validation + inferred types match the UI requirements.
    confirmTnC: z.boolean().refine((value) => value, {
      message: 'Please accept the terms.',
    }),

    confirmSubscription: z.boolean().refine((value) => value, {
      message: 'Please confirm organization subscription.',
    }),
  })

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/billing-details-pages/BillingDetailsPage.tsx
Comment thread src/components/StatefulButton/StatefulSubscribeButton.tsx
Comment thread src/components/StatefulButton/StatefulSubscribeButton.tsx
Comment thread src/components/StatefulButton/StatefulSubscribeButton.tsx
Comment thread src/components/billing-details-pages/BillingDetailsPage.tsx
Copilot AI review requested due to automatic review settings March 18, 2026 05:57
@gshivajibiradar gshivajibiradar force-pushed the ENT-11429-billing-address branch from 1ff8cc8 to f8caed2 Compare March 18, 2026 05:59
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds billing address capture to the Billing Details step and adjusts subscribe-click tracking so the analytics event is emitted reliably from the subscribe button click path.

Changes:

  • Expanded the billing details Zod schema to include full name + Stripe-style address fields.
  • Wired Stripe AddressElement changes into the checkout form/store and passed react-hook-form down to billing form fields.
  • Updated subscribe button click handling and added/updated tests around tracking + new required billing fields.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/constants/checkout.ts Extends BillingDetailsSchema with billing name/address fields for validation.
src/components/billing-details-pages/BillingDetailsPage.tsx Updates form defaults, submit behavior, and subscribe-click tracking hook-up.
src/components/Stepper/StepperContent/BillingDetailsContent.tsx Passes react-hook-form instance into BillingFormFields.
src/components/FormFields/BillingFormFields.tsx Maps Stripe AddressElement change events into RHF + zustand store; renders fullName error feedback.
src/components/StatefulButton/StatefulSubscribeButton.tsx Adds parent onClick hook and updates required-field gating for enabling subscribe.
src/components/billing-details-pages/tests/BillingDetailsPage.test.tsx Seeds store with new required billing address fields and asserts tracking emission on subscribe click.
src/components/StatefulButton/tests/StatefulSubscribeButton.test.tsx Updates test fixture billing data to include new required fields.
src/components/FormFields/tests/BillingFormFields.test.tsx Adds tests for Stripe address mapping and validation feedback rendering.
Comments suppressed due to low confidence (1)

src/constants/checkout.ts:307

  • BillingDetailsSchema validates confirmTnC and confirmSubscription, but the flow also collects confirmRecurringSubscription (see the checkbox UI and StatefulSubscribeButton required fields). Because Zod objects strip unknown keys by default, omitting it here can cause the value to be dropped from the submit payload and then cleared from the zustand store (which replaces the step payload). Add confirmRecurringSubscription to this schema with a required/true refinement to keep validation + persistence consistent.
    confirmSubscription: z.boolean().refine((value) => value, {
      message: 'Please confirm organization subscription.',
    }),
  })

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/billing-details-pages/BillingDetailsPage.tsx
Comment thread src/components/billing-details-pages/BillingDetailsPage.tsx
Comment thread src/components/billing-details-pages/BillingDetailsPage.tsx
Comment thread src/components/StatefulButton/StatefulSubscribeButton.tsx
Comment thread src/components/StatefulButton/StatefulSubscribeButton.tsx
Comment thread src/components/FormFields/tests/BillingFormFields.test.tsx
@gshivajibiradar gshivajibiradar force-pushed the ENT-11429-billing-address branch from f8caed2 to 8020061 Compare March 24, 2026 09:26
import { useCheckoutFormStore, useCurrentPageDetails } from '@/hooks/index';
import { sendEnterpriseCheckoutTrackingEvent } from '@/utils/common';

type BillingDetailsData = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make use of existing BillingDetailsData, do not create new one. Add only additional fields mentioned in ticket,

resolver: zodResolver(billingDetailsSchema),
defaultValues: billingDetailsData,
defaultValues: {
fullName: '',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fullName is already included in billingDetailsData, so there is no need to declare it again

});

setFormData(DataStoreKey.BillingDetails, data);
handleSubmit(onSubmit)().catch(() => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The addition of handleSubscribeClick doesn’t appear to be necessary.
Unless there is additional logic planned inside handleSubscribeClick, it can be removed to avoid unnecessary indirection.

Copilot AI review requested due to automatic review settings March 26, 2026 06:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds required billing address capture to the Billing Details checkout step and adjusts the subscribe click flow so the billing-details subscribe tracking event is emitted reliably, with accompanying test updates.

Changes:

  • Extend Billing Details form/schema to include billing address + full name fields.
  • Update subscribe button click flow to emit the tracking event on click.
  • Add/adjust tests around Billing Details and the new BillingFormFields behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/constants/checkout.ts Expands the Billing Details Zod schema to validate billing address fields.
src/components/billing-details-pages/BillingDetailsPage.tsx Adds default values for new fields and wires subscribe click tracking into the subscribe button.
src/components/billing-details-pages/tests/BillingDetailsPage.test.tsx Seeds store with new required billing fields and updates tracking assertion on subscribe click.
src/components/app/routes/loaders/checkoutStepperLoader.ts Redirects away from Billing Details if the checkout intent is expired or missing a Stripe client secret.
src/components/StripeProvider/StripeProvider.tsx Refactors fetchClientSecret wiring for Stripe CheckoutProvider.
src/components/Stepper/StepperContent/BillingDetailsContent.tsx Passes the RHF form object into BillingFormFields.
src/components/StatefulButton/StatefulSubscribeButton.tsx Adds an optional onClick hook, tightens “required fields present” logic, and improves error handling around patch/confirm.
src/components/StatefulButton/tests/StatefulSubscribeButton.test.tsx Updates mocked store state to include required address fields + recurring subscription confirmation.
src/components/FormFields/BillingFormFields.tsx Integrates Stripe AddressElement change events with RHF + store for billing address capture.
src/components/FormFields/tests/BillingFormFields.test.tsx Adds coverage for Stripe element rendering and address mapping into the store.
src/components/ErrorPage/ErrorPage.tsx Improves message derivation for thrown non-Error objects to avoid [object Object].
Comments suppressed due to low confidence (1)

src/constants/checkout.ts:310

  • BillingDetailsSchema no longer includes confirmRecurringSubscription, but the UI/store logic (e.g. TermsAndConditionsCheckboxes and StatefulSubscribeButton) relies on this field being present and required. This will prevent schema-based validation and Zod-inferred types from matching actual form data; add confirmRecurringSubscription: z.boolean().refine(...) to the schema (and an appropriate error message).
    confirmTnC: z.boolean().refine((value) => value, {
      message: 'Please accept the terms.',
    }),

    confirmSubscription: z.boolean().refine((value) => value, {
      message: 'Please confirm organization subscription.',
    }),
  })

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +83 to +93
const handleSubscribeClick = () => {
sendEnterpriseCheckoutTrackingEvent({
checkoutIntentId: checkoutIntent?.id ?? null,
eventName: EVENT_NAMES.SUBSCRIPTION_CHECKOUT.BILLING_DETAILS_SUBSCRIBE_BUTTON_CLICKED,
eventName:
EVENT_NAMES.SUBSCRIPTION_CHECKOUT
.BILLING_DETAILS_SUBSCRIBE_BUTTON_CLICKED,
});

setFormData(DataStoreKey.BillingDetails, data);
handleSubmit(onSubmit)().catch(() => {
// Form submission errors are handled by react-hook-form
});
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleSubscribeClick invokes handleSubmit(...)() even though StatefulSubscribeButton is rendered with type: 'submit', so clicking it will already trigger the form onSubmit. This can cause duplicate validation/submission work and makes the click flow harder to reason about; consider removing the manual handleSubmit call here, or change the subscribe button to type="button" and explicitly run a single submit/confirm sequence.

Copilot uses AI. Check for mistakes.
Comment on lines +23 to +40
// Wrap fetchClientSecret so that any non-Error rejection from Stripe's SDK
// (e.g. a raw API error object like { type: 'invalid_request_error', ... })
// is converted to a proper Error instance. Without this conversion the
// webpack dev overlay shows the unhelpful "[object Object]" message and
// React Router's error boundary cannot display a meaningful message either.
const fetchClientSecret = (): Promise<string> => (
Promise.resolve(checkoutSessionClientSecret).catch((err: unknown) => {
let message: string;
if (err instanceof Error) {
message = err.message;
} else if (typeof err === 'string') {
message = err;
} else {
message = JSON.stringify(err);
}
throw new Error(`Stripe session initialization failed: ${message}`);
})
);
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetchClientSecret wraps Promise.resolve(checkoutSessionClientSecret) in a .catch(...), but Promise.resolve of a string will never reject, so this conversion logic will never run. If the goal is to surface a helpful message when the secret is missing/invalid, validate and throw before resolving (e.g., if the secret is falsy) or handle Stripe initialization/confirm errors at the callsite/error boundary instead of relying on this .catch.

Suggested change
// Wrap fetchClientSecret so that any non-Error rejection from Stripe's SDK
// (e.g. a raw API error object like { type: 'invalid_request_error', ... })
// is converted to a proper Error instance. Without this conversion the
// webpack dev overlay shows the unhelpful "[object Object]" message and
// React Router's error boundary cannot display a meaningful message either.
const fetchClientSecret = (): Promise<string> => (
Promise.resolve(checkoutSessionClientSecret).catch((err: unknown) => {
let message: string;
if (err instanceof Error) {
message = err.message;
} else if (typeof err === 'string') {
message = err;
} else {
message = JSON.stringify(err);
}
throw new Error(`Stripe session initialization failed: ${message}`);
})
);
// Provide fetchClientSecret in the shape expected by Stripe's SDK.
// We defensively validate the client secret and surface a helpful error
// if it is missing or invalid.
const fetchClientSecret = async (): Promise<string> => {
if (!checkoutSessionClientSecret) {
throw new Error('Stripe session initialization failed: missing client secret');
}
return checkoutSessionClientSecret;
};

Copilot uses AI. Check for mistakes.
Comment on lines +80 to +84
try {
return JSON.stringify(errObj, null, 2);
} catch {
return '[Unknown error]';
}
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getErrorMessage falls back to pretty-printing the entire thrown object via JSON.stringify(errObj, null, 2) and rendering it to end users. Depending on what is thrown (e.g., API/Stripe error payloads), this can expose sensitive details; prefer returning a safe generic message (or only stringify in non-production environments), and keep full details in logs/telemetry instead.

Suggested change
try {
return JSON.stringify(errObj, null, 2);
} catch {
return '[Unknown error]';
}
// In non-production, include a pretty-printed version to aid debugging.
if (process.env.NODE_ENV !== 'production') {
try {
return JSON.stringify(errObj, null, 2);
} catch {
// fall through to generic message below
}
}
// In production (or if stringification fails), avoid exposing raw error details.
return '[Unknown error]';

Copilot uses AI. Check for mistakes.
Comment on lines +218 to +224
StatefulSubscribeButton.propTypes = {
onClick: PropTypes.func,
};

StatefulSubscribeButton.defaultProps = {
onClick: undefined,
};
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This TS component adds propTypes/defaultProps for an optional prop that is already fully described by StatefulSubscribeButtonProps. The rest of the codebase appears to rely on TypeScript typing (no other .propTypes usages), so this adds redundant maintenance overhead; consider removing the prop-types dependency and these assignments.

Copilot uses AI. Check for mistakes.
Comment on lines +193 to +228
it('updates form setValue for all address fields when address changes', async () => {
const user = userEvent.setup();
const setValueSpy = jest.fn();

const Wrapper = () => {
const form = useForm<BillingDetailsData>({
mode: 'onTouched',
defaultValues: {
fullName: '',
country: '',
line1: '',
line2: '',
city: '',
state: '',
zip: '',
},
});

// Spy on the setValue method
const originalSetValue = form.setValue;
form.setValue = jest.fn(originalSetValue);
setValueSpy.mockImplementation(form.setValue);

return (
<IntlProvider locale="en">
<BillingFormFields form={form} />
</IntlProvider>
);
};

render(<Wrapper />);
await user.click(screen.getByTestId('address-element'));

expect(mockSetFormData).toHaveBeenCalled();
});

Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case sets up setValueSpy and overrides form.setValue, but never asserts that setValue was called (and setValueSpy isn't used). As written, it only duplicates the earlier setFormData assertion and adds dead code; either assert the expected setValue calls or remove this test/setup.

Suggested change
it('updates form setValue for all address fields when address changes', async () => {
const user = userEvent.setup();
const setValueSpy = jest.fn();
const Wrapper = () => {
const form = useForm<BillingDetailsData>({
mode: 'onTouched',
defaultValues: {
fullName: '',
country: '',
line1: '',
line2: '',
city: '',
state: '',
zip: '',
},
});
// Spy on the setValue method
const originalSetValue = form.setValue;
form.setValue = jest.fn(originalSetValue);
setValueSpy.mockImplementation(form.setValue);
return (
<IntlProvider locale="en">
<BillingFormFields form={form} />
</IntlProvider>
);
};
render(<Wrapper />);
await user.click(screen.getByTestId('address-element'));
expect(mockSetFormData).toHaveBeenCalled();
});

Copilot uses AI. Check for mistakes.
Comment on lines +101 to +105
{form.formState.errors?.fullName?.message && (
<Form.Control.Feedback type="invalid" hasIcon={false} className="d-block mt-2">
{form.formState.errors.fullName.message}
</Form.Control.Feedback>
)}
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only fullName validation feedback is rendered, but the schema now requires other address fields (country, line1, city, state, zip). If the AddressElement doesn’t populate these (or is incomplete), the user may be blocked from submitting with no visible error. Consider adding a generalized address error message when any of those related fields have validation errors (or integrating validation feedback with the AddressElement).

Copilot uses AI. Check for mistakes.
Comment on lines +26 to +34
type BillingDetailsData = {
fullName?: string;
country?: string;
line1?: string;
line2?: string;
city?: string;
state?: string;
zip?: string;
};
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file defines a local BillingDetailsData type that diverges from the app-wide BillingDetailsData (derived from BillingDetailsSchema) and omits checkbox fields like confirmTnC / confirmSubscription / confirmRecurringSubscription. This can lead to incorrect typing for useForm values and setFormData payloads; prefer using the shared type (or extending it) instead of redefining it locally.

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings March 26, 2026 08:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

src/components/billing-details-pages/BillingDetailsPage.tsx:115

  • On the Essentials billing-details route, this Back button still navigates to CheckoutPageRoute.AccountDetails (the non-essentials path). Since the page is now rendered for /essentials/billing-details as well, this will send users to the wrong flow (/account-details). Consider using a route value derived from the current pathname (essentials vs non-essentials), using navigate(-1), or selecting between EssentialsPageRoute.AccountDetails and CheckoutPageRoute.AccountDetails based on the active flow.
            <Button
              variant="outline-primary"
              onClick={() => navigate(CheckoutPageRoute.AccountDetails)}
            >

src/constants/checkout.ts:310

  • BillingDetailsSchema is missing confirmRecurringSubscription, but the billing UI and subscribe-button logic both depend on that field (e.g., TermsAndConditionsCheckboxes registers it and StatefulSubscribeButton treats it as required). Because Zod objects strip unknown keys by default, submissions/resolver output can drop this value, and setFormData replacements can inadvertently clear the checkbox state. Add confirmRecurringSubscription to the schema (with the same “must be true” refinement/message pattern as the other confirmations) so form validation and submission data stay consistent.
export const BillingDetailsSchema = (constraints: CheckoutContextFieldConstraints) => (
  z.object({
    fullName: z.string().trim()
      .min(
        constraints?.fullName?.minLength ?? 1,
        'Please provide your full name.',
      )
      .max(
        constraints?.fullName?.maxLength ?? 150,
        `Name is too long. It must contain no more than ${constraints?.fullName?.maxLength ?? 150} characters.`,
      ),

    country: z.string().trim()
      .min(
        constraints?.country?.minLength ?? 2,
        'Country is required',
      ),

    line1: z.string().trim()
      .min(1, 'Address line 1 is required'),

    line2: z.string().trim().optional(),

    city: z.string().trim()
      .min(1, 'City is required'),

    state: z.string().trim()
      .min(1, 'State is required'),

    zip: z.string().trim()
      .min(1, 'ZIP code is required'),

    confirmTnC: z.boolean().refine((value) => value, {
      message: 'Please accept the terms.',
    }),

    confirmSubscription: z.boolean().refine((value) => value, {
      message: 'Please confirm organization subscription.',
    }),
  })

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +23 to +41
// Wrap fetchClientSecret so that any non-Error rejection from Stripe's SDK
// (e.g. a raw API error object like { type: 'invalid_request_error', ... })
// is converted to a proper Error instance. Without this conversion the
// webpack dev overlay shows the unhelpful "[object Object]" message and
// React Router's error boundary cannot display a meaningful message either.
const fetchClientSecret = (): Promise<string> => (
Promise.resolve(checkoutSessionClientSecret).catch((err: unknown) => {
let message: string;
if (err instanceof Error) {
message = err.message;
} else if (typeof err === 'string') {
message = err;
} else {
message = JSON.stringify(err);
}
throw new Error(`Stripe session initialization failed: ${message}`);
})
);

Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetchClientSecret wraps Promise.resolve(checkoutSessionClientSecret).catch(...), but useCheckoutSessionClientSecret() is typed/implemented to return string | null, so this promise won’t reject in normal operation. If the goal is to normalize Stripe initialization failures (including non-Error rejections), this wrapper likely won’t intercept them as written. Consider either simplifying this back to a straightforward resolved promise, or moving the normalization to where the rejection actually occurs (or updating the hook/contract so checkoutSessionClientSecret can legitimately be a promise/thenable).

Suggested change
// Wrap fetchClientSecret so that any non-Error rejection from Stripe's SDK
// (e.g. a raw API error object like { type: 'invalid_request_error', ... })
// is converted to a proper Error instance. Without this conversion the
// webpack dev overlay shows the unhelpful "[object Object]" message and
// React Router's error boundary cannot display a meaningful message either.
const fetchClientSecret = (): Promise<string> => (
Promise.resolve(checkoutSessionClientSecret).catch((err: unknown) => {
let message: string;
if (err instanceof Error) {
message = err.message;
} else if (typeof err === 'string') {
message = err;
} else {
message = JSON.stringify(err);
}
throw new Error(`Stripe session initialization failed: ${message}`);
})
);
const fetchClientSecret = async (): Promise<string> => checkoutSessionClientSecret;

Copilot uses AI. Check for mistakes.
Comment on lines +193 to +227
it('updates form setValue for all address fields when address changes', async () => {
const user = userEvent.setup();
const setValueSpy = jest.fn();

const Wrapper = () => {
const form = useForm<BillingDetailsData>({
mode: 'onTouched',
defaultValues: {
fullName: '',
country: '',
line1: '',
line2: '',
city: '',
state: '',
zip: '',
},
});

// Spy on the setValue method
const originalSetValue = form.setValue;
form.setValue = jest.fn(originalSetValue);
setValueSpy.mockImplementation(form.setValue);

return (
<IntlProvider locale="en">
<BillingFormFields form={form} />
</IntlProvider>
);
};

render(<Wrapper />);
await user.click(screen.getByTestId('address-element'));

expect(mockSetFormData).toHaveBeenCalled();
});
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test "updates form setValue for all address fields" sets up setValueSpy, but never asserts that setValue was called for the individual address fields. As written, it only re-checks that setFormData was called (which is already covered by other tests), so it won’t fail if form.setValue updates regress. Add explicit expectations for form.setValue calls (or remove the unused spy/test if it’s redundant).

Copilot uses AI. Check for mistakes.
} = form;

const onSubmit = async (data: BillingDetailsData) => {
setFormData(DataStoreKey.BillingDetails, data);
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setFormData replaces the entire BillingDetails payload (it does not deep-merge). Submitting the form with setFormData(DataStoreKey.BillingDetails, data) can therefore wipe previously stored fields (e.g., the T&C checkbox values) if the resolver output doesn’t include them. Consider merging with the existing store value (e.g., spread billingDetailsData into the object you store) or ensure the schema/resolver output includes all BillingDetails fields that must be preserved.

Suggested change
setFormData(DataStoreKey.BillingDetails, data);
setFormData(
DataStoreKey.BillingDetails,
{
...(billingDetailsData || {}),
...data,
},
);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants