28 checkout flow#74
Merged
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCentralizes checkout state in a new useCheckoutFlow hook, adds auth UI (auth selector, login, create-account) and a reusable form input, and refactors the checkout route and ConfirmationPanel to use the flow for modal and claim orchestration. ChangesAuthentication Modals & Inputs
Sequence DiagramsequenceDiagram
participant User
participant CheckoutPage
participant CheckoutFlow
participant AuthModal
participant ConfirmModal
participant API as Mutations
participant Nav as Navigation
User->>CheckoutPage: Click "Claim Gifts"
CheckoutPage->>CheckoutFlow: flow.start()
alt unauthenticated
CheckoutFlow->>CheckoutPage: authModalOpen = true
CheckoutPage->>AuthModal: render CheckoutAuthModal
User->>AuthModal: Select login or register
AuthModal->>CheckoutFlow: flow.submitLogin / flow.submitRegister
CheckoutFlow->>API: Login/Register mutation
API-->>CheckoutFlow: Success/Error
CheckoutFlow->>CheckoutFlow: confirmClaim()
else authenticated donor
CheckoutFlow->>CheckoutPage: confirmModalOpen = true
CheckoutPage->>ConfirmModal: render with isPending
end
User->>ConfirmModal: Confirm
ConfirmModal->>CheckoutFlow: flow.confirmClaim()
CheckoutFlow->>API: ClaimGifts mutation
API-->>CheckoutFlow: Claimed
CheckoutFlow->>Nav: navigate to /donor/home
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
app/src/components/storefront/CheckoutFieldInput.tsx (1)
14-28: Defaulttypeso the rendered input never hastype={undefined}.When the consumer omits
type,inputTypeevaluates toundefinedand is forwarded to<Input>. It works (browser defaults to"text"), but it makes the prop signature ambiguous and breaks features like password autofill / numeric keyboards if the consumer simply forgot to pass it. A small default makes the contract explicit and keepsisPasswordderivation clean.♻️ Proposed refactor
export function CheckoutFieldInput({ - type, + type = "text", field, placeholder, disabled, startIcon, }: CheckoutFieldInputProps) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/storefront/CheckoutFieldInput.tsx` around lines 14 - 28, The CheckoutFieldInput component lets the input `type` be undefined which causes `inputType` to become undefined and be forwarded to <Input>, so set a sensible default for the `type` prop (e.g., default to "text") in the CheckoutFieldInput signature or destructuring to ensure `type` is never undefined; update the derivation of `isPassword` / `inputType` (symbols: CheckoutFieldInput, type, isPassword, inputType) to rely on that default and keep password toggle logic intact.app/src/components/storefront/CheckoutAuthModal.tsx (1)
46-50: Use strict equality.Prefer
===over==for theauthModecomparisons.- {flow.authMode == "login" ? "User Login" : "Create Account"} + {flow.authMode === "login" ? "User Login" : "Create Account"} ... - {flow.authMode == "login" ? ( + {flow.authMode === "login" ? (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/storefront/CheckoutAuthModal.tsx` around lines 46 - 50, In CheckoutAuthModal replace the loose equality checks on flow.authMode with strict equality (use === instead of ==) for every comparison (e.g., the ternary that renders the header and the conditional that chooses the login vs create-account block) so the component uses strict type-safe comparisons against "login".app/src/components/storefront/CheckoutCreateAccountModal.tsx (1)
143-189: Avoid side-effects inside the validator.
setPasswordCriteriasis called from withinvalidators.onChange, mixing validation with imperative React state updates. Prefer deriving the checklist from the live password value viaform.Subscribe/useStore, which keeps the validator pure and avoids redundant renders. The criteria block itself can also be condensed:- if (value.length < 8) newCriterias[0] = false; - else newCriterias[0] = true; - - if (!/[A-Z]/.test(value)) newCriterias[1] = false; - else newCriterias[1] = true; - - if (!/[a-z]/.test(value)) newCriterias[2] = false; - else newCriterias[2] = true; - - if (!/\d/.test(value)) newCriterias[3] = false; - else newCriterias[3] = true; - - if (!/[@$!%*?&#^()]/.test(value)) newCriterias[4] = false; - else newCriterias[4] = true; + newCriterias[0] = value.length >= 8; + newCriterias[1] = /[A-Z]/.test(value); + newCriterias[2] = /[a-z]/.test(value); + newCriterias[3] = /\d/.test(value); + newCriterias[4] = /[@$!%*?&#^()]/.test(value);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/storefront/CheckoutCreateAccountModal.tsx` around lines 143 - 189, The validator for the password field in CheckoutCreateAccountModal (validators.onChange on the form.Field "password") is performing a side-effect by calling setPasswordCriterias; remove that state mutation from the validator so the validator is a pure function that only returns validation errors. Move the checklist derivation into a subscription or effect (e.g., form.Subscribe or useStore / useEffect that watches the "password" field value) and there compute the five boolean criteria (length, uppercase, lowercase, digit, special char) in a compact map of regex tests and call setPasswordCriterias from that subscription; keep validators.onChange to only run the same tests and return undefined or the error string, but do not call setPasswordCriterias there.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/storefront/CheckoutAuthModal.tsx`:
- Line 49: In the CheckoutAuthModal component update the logo <img> that uses
the KFKLogo (the JSX element currently rendered as <img src={KFKLogo}
className="w-75 mx-auto mb-5" />) to include an appropriate alt attribute—either
a meaningful description like alt="KFK logo" if relevant to users or alt="" if
it is purely decorative—so screen readers won't announce the filename.
- Around line 16-20: The JSX in CheckoutAuthModal contains a nested <p> (outer
paragraph with an inner <p>), which is invalid; update the markup in the
CheckoutAuthModal component so the inner element is not a <p> (e.g., replace the
inner <p> with a <span> or plain text with a <br/>), ensuring the outer
paragraph remains a single valid <p> element and the visual text/line break is
preserved.
In `@app/src/components/storefront/CheckoutCreateAccountModal.tsx`:
- Around line 197-215: The confirmPassword field's validator (in the form.Field
for name "confirmPassword") only runs on the confirm field change; add
onChangeListenTo: ["password"] to its validators configuration so the
confirmPassword validator re-runs when the "password" field changes, ensuring
cross-field validation updates in real time; update the validators object on the
form.Field named "confirmPassword" (the same place that currently defines
onChange) to include onChangeListenTo: ["password"] alongside the existing
onChange handler.
- Around line 217-296: The selector in form.Subscribe returns four values but
the render destructures three, causing misalignment; update the selector to
return only [state.canSubmit, state.fieldMeta.password?.isTouched,
state.fieldMeta.password?.isDirty] (remove state.isSubmitting) so the children
destructure ([canSubmit, isTouched, isDirty]) correctly and isPasswordPristine =
!isTouched && !isDirty works as intended; locate form.Subscribe, the selector
function, and the destructuring block around isPasswordPristine to make this
change.
In `@app/src/components/storefront/CheckoutFieldInput.tsx`:
- Around line 38-70: The input needs accessible wiring so screen readers
announce validations: when errorMessage is present, set aria-invalid="true" on
the <Input> (component using props in CheckoutFieldInput) and add
aria-describedby pointing to a unique id for the error span (e.g.,
`${field.id}-error`); then give the error <span> that id and add role="alert"
(or aria-live="polite") so the message is announced when it appears. Update the
Input props to include aria-invalid and aria-describedby conditionally based on
errorMessage, and update the error span to include the matching id and
role/aria-live.
- Around line 38-65: The password-eye overlaps input text because the Input's
className only adds left padding (pl-8) for the start icon and not right padding
for the absolutely-positioned toggle; update the Input's className in
CheckoutFieldInput (the Input element rendered near isPassword / showPassword
and the Button toggle that calls setShowPassword) to conditionally add right
padding (e.g., pr-8 or pr-10) when isPassword is true so the typed text doesn't
flow under the Eye toggle and touch targets remain on the input.
- Around line 51-64: The password visibility toggle (Button in
CheckoutFieldInput, wrapped by isPassword and using setShowPassword/showPassword
with Eye/EyeOff) is currently icon-only and lacks accessibility attributes;
update the Button to keep type="button" and add an accessible name and state by
setting aria-label dynamically to "Show password" when showPassword is false and
"Hide password" when true, and include aria-pressed={showPassword} (or
role="switch" + aria-checked if preferred) so screen readers get both the label
and current state; ensure the visual icon rendering (Eye/EyeOff) stays the same
while these ARIA attributes provide the accessible semantics.
- Around line 6-12: Change the CheckoutFieldInputProps type to use the exported
AnyFieldApi from `@tanstack/react-form` instead of any: replace the field: any
property with field: AnyFieldApi and add an import for AnyFieldApi from
'@tanstack/react-form'. Update any usages of CheckoutFieldInputProps/field (in
the CheckoutFieldInput component) to accept the new type without other code
changes.
In `@app/src/hooks/useCheckoutFlow.ts`:
- Around line 74-99: submitLogin and submitRegister call confirmClaim() with a
stale captured auth and thus bypass the donor-role UI gate; update both to read
fresh session/auth after successful auth (e.g., use the resolved loginMutation
result or call queryClient.getQueryData(queries.session.verify.queryKey)) and
only call confirmClaim() if the current auth.role === UserRole.DONOR, otherwise
surface the disabledMessage; also adjust submitRegister's error handling to
distinguish failures from register vs login (report the actual error from the
step that threw rather than always "Failed to register").
---
Nitpick comments:
In `@app/src/components/storefront/CheckoutAuthModal.tsx`:
- Around line 46-50: In CheckoutAuthModal replace the loose equality checks on
flow.authMode with strict equality (use === instead of ==) for every comparison
(e.g., the ternary that renders the header and the conditional that chooses the
login vs create-account block) so the component uses strict type-safe
comparisons against "login".
In `@app/src/components/storefront/CheckoutCreateAccountModal.tsx`:
- Around line 143-189: The validator for the password field in
CheckoutCreateAccountModal (validators.onChange on the form.Field "password") is
performing a side-effect by calling setPasswordCriterias; remove that state
mutation from the validator so the validator is a pure function that only
returns validation errors. Move the checklist derivation into a subscription or
effect (e.g., form.Subscribe or useStore / useEffect that watches the "password"
field value) and there compute the five boolean criteria (length, uppercase,
lowercase, digit, special char) in a compact map of regex tests and call
setPasswordCriterias from that subscription; keep validators.onChange to only
run the same tests and return undefined or the error string, but do not call
setPasswordCriterias there.
In `@app/src/components/storefront/CheckoutFieldInput.tsx`:
- Around line 14-28: The CheckoutFieldInput component lets the input `type` be
undefined which causes `inputType` to become undefined and be forwarded to
<Input>, so set a sensible default for the `type` prop (e.g., default to "text")
in the CheckoutFieldInput signature or destructuring to ensure `type` is never
undefined; update the derivation of `isPassword` / `inputType` (symbols:
CheckoutFieldInput, type, isPassword, inputType) to rely on that default and
keep password toggle logic intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2343511-d6f0-4e97-96c2-14bf75a1f1a1
📒 Files selected for processing (7)
app/src/components/storefront/CheckoutAuthModal.tsxapp/src/components/storefront/CheckoutCreateAccountModal.tsxapp/src/components/storefront/CheckoutFieldInput.tsxapp/src/components/storefront/CheckoutLoginModal.tsxapp/src/components/storefront/ConfirmationPanel.tsxapp/src/hooks/useCheckoutFlow.tsapp/src/routes/_storefront/checkout.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/hooks/useCheckoutFlow.ts (1)
73-97:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAwait the auth mutations to honor the async contract.
submitLogin()and the login portion ofsubmitRegister()declare async but return immediately becauseloginMutation.mutate()is callback-based and not awaited. Callers expecting aPromise<void>will resume before authentication andconfirmClaim()complete.Convert both to
mutateAsync()with try/catch:Proposed fix
const submitLogin = async (email: string, password: string) => { - loginMutation.mutate( - { email, password }, - { - onSuccess: async (session) => { - if (!session || session.role !== UserRole.DONOR) { - setAuthModalOpen(false); - setDisabledMessage( - "Only donors can claim gifts. Please log in with a donor account.", - ); - return; - } - - setAuthModalOpen(false); - await confirmClaim(); - }, - onError: (error) => { - const message = - error instanceof Error ? error.message : "Failed to login"; - console.log(message); - toast.error("Failed to login. Check your username and password."); - }, - }, - ); + try { + const session = await loginMutation.mutateAsync({ email, password }); + + if (!session || session.role !== UserRole.DONOR) { + setAuthModalOpen(false); + setDisabledMessage( + "Only donors can claim gifts. Please log in with a donor account.", + ); + return; + } + + setAuthModalOpen(false); + await confirmClaim(); + } catch (error) { + const message = + error instanceof Error ? error.message : "Failed to login"; + console.error(message); + toast.error("Failed to login. Check your username and password."); + } }; const submitRegister = async (data: RegisterDonorInput) => { try { await registerMutation.mutateAsync({ data }); - loginMutation.mutate( - { - email: data.email, - password: data.password, - }, - { - onSuccess: async (session) => { - if (!session || session.role !== UserRole.DONOR) { - setAuthModalOpen(false); - setDisabledMessage( - "Only donors can claim gifts. Please log in with a donor account.", - ); - return; - } - - await confirmClaim(); - }, - onError: (error) => { - const message = - error instanceof Error - ? error.message - : "Failed to register or login"; - console.error(message); - toast.error("Failed to login"); - }, - }, - ); + const session = await loginMutation.mutateAsync({ + email: data.email, + password: data.password, + }); + + if (!session || session.role !== UserRole.DONOR) { + setAuthModalOpen(false); + setDisabledMessage( + "Only donors can claim gifts. Please log in with a donor account.", + ); + return; + } + + setAuthModalOpen(false); + await confirmClaim(); } catch (error) { const message = error instanceof Error ? error.message : "Failed to register or login"; console.error(message); toast.error("Failed to register or login"); } };Applies to lines 73–97 and 99–135.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/hooks/useCheckoutFlow.ts` around lines 73 - 97, submitLogin and the register flow use the callback-based loginMutation.mutate/registerMutation.mutate so they return immediately despite being declared async; change both flows (submitLogin and the login portion of submitRegister) to call await loginMutation.mutateAsync(...) and await registerMutation.mutateAsync(...) inside a try/catch so callers actually wait for authentication and confirmClaim to complete, preserve the existing onSuccess logic (check session && session.role === UserRole.DONOR, setAuthModalOpen(false), call await confirmClaim(), setDisabledMessage on non-donor) and in the catch block log the error and show the toast (use error instanceof Error ? error.message : "Failed to login"/"Failed to register" as before).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/hooks/useCheckoutFlow.ts`:
- Around line 108-117: In the onSuccess handler (the branch that currently
checks session and UserRole.DONOR), close the auth modal before calling
confirmClaim() so the UI matches submitLogin() behavior: call
setAuthModalOpen(false) prior to awaiting confirmClaim() to ensure the modal is
dismissed even if confirmClaim() fails; keep the existing role check and
setDisabledMessage flow intact and only reorder the calls in onSuccess to
setAuthModalOpen(false) then await confirmClaim().
---
Duplicate comments:
In `@app/src/hooks/useCheckoutFlow.ts`:
- Around line 73-97: submitLogin and the register flow use the callback-based
loginMutation.mutate/registerMutation.mutate so they return immediately despite
being declared async; change both flows (submitLogin and the login portion of
submitRegister) to call await loginMutation.mutateAsync(...) and await
registerMutation.mutateAsync(...) inside a try/catch so callers actually wait
for authentication and confirmClaim to complete, preserve the existing onSuccess
logic (check session && session.role === UserRole.DONOR,
setAuthModalOpen(false), call await confirmClaim(), setDisabledMessage on
non-donor) and in the catch block log the error and show the toast (use error
instanceof Error ? error.message : "Failed to login"/"Failed to register" as
before).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2676ba36-e589-4655-ac41-66b2f65c119f
📒 Files selected for processing (5)
app/src/components/storefront/CheckoutAuthModal.tsxapp/src/components/storefront/CheckoutCreateAccountModal.tsxapp/src/components/storefront/CheckoutFieldInput.tsxapp/src/hooks/useCheckoutFlow.tsapp/src/routes/_storefront/checkout.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- app/src/components/storefront/CheckoutCreateAccountModal.tsx
- app/src/routes/_storefront/checkout.tsx
- app/src/components/storefront/CheckoutFieldInput.tsx
- app/src/components/storefront/CheckoutAuthModal.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Description
Built the checkout flow for donors to claim gifts. Built CheckoutAuthModal for register/login popup and useCheckoutFlow hook to manage states.
Type of Change
Related Issues (put task name here from notion)
Screenshots (If it is a front end feature screenshot is required)
Log in and create account pop up when user pressed claim gift without being logged into an account:
Error Toast when gift becomes unavilable:
Additional Notes
Summary by CodeRabbit
New Features
User Experience
Bug Fixes / Behavior