feat: PlanDetails page now uses a StatefulButton and creates a CheckoutIntent #60
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a stateful button component for the PlanDetails page and integrates checkout intent creation functionality. The changes enable the form to create a CheckoutIntent via API call when submitted by authenticated users, while maintaining existing logistration flows for unauthenticated users.
- Adds a new
PlanDetailsSubmitButtoncomponent that usesStatefulButtonwith different states (inactive, default, pending, complete, errored) - Integrates checkout intent creation API service and mutation hook for authenticated users
- Updates the PlanDetailsPage to use the new button and handle checkout intent creation workflow
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| PlanDetailsSubmitButton.tsx | New stateful button component with visual feedback for different submission states |
| PlanDetailsPage.tsx | Updated to use new button component and integrate checkout intent creation logic |
| checkout-intent.ts | New service for creating checkout intents via API |
| useCreateCheckoutIntentMutation.tsx | New mutation hook for checkout intent creation |
| hooks/index.ts | Export new mutation hook and another existing hook |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| inactive: buttonMessage!, | ||
| default: buttonMessage!, |
There was a problem hiding this comment.
Using non-null assertion operator (!) is risky here. The buttonMessage from useCurrentPageDetails() could potentially be undefined, which would cause runtime errors. Consider adding a fallback value or proper null checking.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
@pwnage101 I would follow up with this AI comment
| }; | ||
|
|
||
| const onSubmit = (data: PlanDetailsData) => onSubmitCallbacks[currentPage!](data); | ||
| const onSubmit = (data: any) => onSubmitCallbacks[currentPage!](data); |
There was a problem hiding this comment.
Using any type removes type safety. The original type PlanDetailsData should be maintained, or a union type should be used to handle the different form data types (PlanDetailsData | PlanDetailsLoginPageData | PlanDetailsRegisterPageData).
| const onSubmit = (data: any) => onSubmitCallbacks[currentPage!](data); | |
| const onSubmit = (data: PlanDetailsData | PlanDetailsLoginPageData | PlanDetailsRegisterPageData) => onSubmitCallbacks[currentPage!](data); |
Copilot uses AI. Check for mistakes.
| // @ts-ignore(TS2339) | ||
| transformResponse: axios.defaults.transformResponse!.concat(camelCaseResponse), |
There was a problem hiding this comment.
Using @ts-ignore suppresses TypeScript errors and should be avoided. Consider properly typing the transformResponse property or using a type assertion with proper typing instead.
| // @ts-ignore(TS2339) | |
| transformResponse: axios.defaults.transformResponse!.concat(camelCaseResponse), | |
| transformResponse: axios.defaults.transformResponse!.concat(camelCaseResponse) as import('axios').AxiosRequestConfig['transformResponse'], |
Copilot uses AI. Check for mistakes.
| } | ||
| }, [formIsValid, submissionIsPending, submissionIsSuccess, submissionIsError]); | ||
|
|
||
| const allButtonMessages: Record<ButtonState, MessageDescriptor> = { |
There was a problem hiding this comment.
[nit] This extra constant may not be necessary. The consuming StatefulButton can reference the defineMessages object directly or buttonMessage from useCurrentPageDetails
| inactive: undefined, | ||
| default: undefined, |
There was a problem hiding this comment.
[nit] These can be removed.
| inactive: buttonMessage!, | ||
| default: buttonMessage!, |
There was a problem hiding this comment.
@pwnage101 I would follow up with this AI comment
| } | ||
|
|
||
| // If we already have an intent or the mutation succeeded earlier, just proceed. | ||
| if (existingCheckoutIntent || createCheckoutIntentMutation.isSuccess) { |
There was a problem hiding this comment.
[nit] is the check for createCheckoutIntentMutation.isSuccess necessary for redirect if the onSuccess logic redirects to the account details?
pwnage101
force-pushed
the
pwnage101/ENT-10756
branch
2 times, most recently
from
459e177 to
b69daca
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
pwnage101
force-pushed
the
pwnage101/ENT-10756
branch
2 times, most recently
from
1b4b5d4 to
0ba7f5e
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
pwnage101
force-pushed
the
pwnage101/ENT-10756
branch
2 times, most recently
from
86e14e1 to
eda0085
Compare
…utIntent ENT-10756
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
pwnage101
force-pushed
the
pwnage101/ENT-10756
branch
from
eda0085 to
5bd6fe0
Compare
2 participants
ENT-10756
Pairs with:
plan_details_stateful_button.mov