[TASK-14062] persist dynamic bank account form state#1157
[TASK-14062] persist dynamic bank account form state#1157Zishan-7 merged 5 commits intopeanut-wallet-devfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a new Redux slice (bankForm) to persist/clear DynamicBankAccountForm data, wires it into the store and constants, preloads persisted data into the form, saves trimmed form values on successful submit, and clears persisted form state on back navigation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
src/redux/types/bank-form.types.ts (1)
1-7: Prefer type-only import and decouple UI-layer types.Use
import typeto avoid unintended runtime imports, and consider movingIBankAccountDetailsto a shared types module to prevent cross-layer coupling.Apply:
-import { IBankAccountDetails } from '@/components/AddWithdraw/DynamicBankAccountForm' +import type { IBankAccountDetails } from '@/components/AddWithdraw/DynamicBankAccountForm'Optional (separate change): create
src/types/bank-account.types.tsforIBankAccountDetailsand import from there to avoid Redux↔UI dependency.src/components/AddWithdraw/DynamicBankAccountForm.tsx (2)
76-78: Avoid tight coupling to state shape; add a selector.Directly reaching into
state.bankForm.formDatacouples this component to slice placement. Prefer a selector to decouple and centralize typing.Example selector (place under
src/redux/selectors/bankForm.ts):import { RootState } from '@/redux/store' export const selectBankFormData = (state: RootState) => state.bankForm.formDataThen in this component:
-const persistedFormData = useAppSelector((state) => state.bankForm.formData) +const persistedFormData = useAppSelector(selectBankFormData)
99-100: defaultValues won’t refresh after mount; reset on persisted changes if needed.If
persistedFormDatacan change after first render (e.g., navigating back), callresetwhen it changes; otherwise fields may show stale values.// add to useForm destructure const { control, handleSubmit, setValue, getValues, reset, formState: { ... } } = useForm<IBankAccountDetails>(...) // rehydrate on persisted changes useEffect(() => { if (persistedFormData) reset({ ...getValues(), ...persistedFormData }) // eslint-disable-next-line react-hooks/exhaustive-deps }, [persistedFormData])src/redux/slices/bank-form-slice.ts (5)
17-29: Type flexibility and safety forupdateFormField.
valueis typed asstring, which is fine today but ties reducer to string-only fields. Consider inferring from key for future-proofing.Example:
- action: PayloadAction<{ - field: keyof IBankAccountDetails - value: string - }> + action: PayloadAction<{ + field: keyof IBankAccountDetails + value: IBankAccountDetails[keyof IBankAccountDetails] + }>
4-4: Decouple slice from component-level types.Importing
IBankAccountDetailsfrom a component path creates UI↔state coupling and risks circular deps. Move the type to a shared domain types module (e.g.,src/types/bank.ts) and import from there.
30-32: Clear vs. empty object—be explicit about intent.If “no data” is semantically different from “some fields empty”,
nullis fine. If not, using{}simplifies merges and avoids null checks elsewhere. Keep as-is if other code relies onnull.
36-37: Export selectors to reduce component coupling.Provide
selectBankFormData(and others if needed) alongside actions to standardize access.Example (add below reducer):
// selectors export const selectBankFormData = (state: RootState) => state.bankForm.formData
1-38: PII in Redux—confirm it’s ephemeral and not persisted.You’re storing account numbers, CLABE, routing numbers, etc. Ensure:
- No redux-persist/localStorage for this slice.
- No logging of actions/state in production.
- Data is cleared on route leave/back (as PR notes).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/components/AddWithdraw/AddWithdrawCountriesList.tsx(3 hunks)src/components/AddWithdraw/DynamicBankAccountForm.tsx(5 hunks)src/components/Claim/Link/views/BankFlowManager.view.tsx(3 hunks)src/redux/constants/index.ts(1 hunks)src/redux/slices/bank-form-slice.ts(1 hunks)src/redux/store.ts(2 hunks)src/redux/types/bank-form.types.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2024-10-25T11:33:46.776Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#484
File: src/components/Cashout/Components/Initial.view.tsx:273-274
Timestamp: 2024-10-25T11:33:46.776Z
Learning: In the `InitialCashoutView` component (`src/components/Cashout/Components/Initial.view.tsx`), linked bank accounts should not generate error states, and the `ValidatedInput` component will clear any error messages if needed. Therefore, it's unnecessary to manually clear the error state when selecting or clearing linked bank accounts.
Applied to files:
src/components/Claim/Link/views/BankFlowManager.view.tsxsrc/components/AddWithdraw/AddWithdrawCountriesList.tsx
📚 Learning: 2025-05-15T14:47:26.891Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#857
File: src/hooks/useWebSocket.ts:77-82
Timestamp: 2025-05-15T14:47:26.891Z
Learning: The useWebSocket hook in src/hooks/useWebSocket.ts is designed to provide raw history entries, while the components using it (such as HomeHistory.tsx) are responsible for implementing deduplication logic based on UUID to prevent duplicate entries when combining WebSocket data with other data sources.
Applied to files:
src/components/Claim/Link/views/BankFlowManager.view.tsx
📚 Learning: 2025-05-22T15:38:48.586Z
Learnt from: kushagrasarathe
PR: peanutprotocol/peanut-ui#869
File: src/app/(mobile-ui)/withdraw/page.tsx:82-88
Timestamp: 2025-05-22T15:38:48.586Z
Learning: The country-specific withdrawal route exists at src/app/(mobile-ui)/withdraw/[...country]/page.tsx and renders the AddWithdrawCountriesList component with flow="withdraw".
Applied to files:
src/components/AddWithdraw/AddWithdrawCountriesList.tsxsrc/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2025-08-14T14:42:54.411Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1094
File: src/utils/withdraw.utils.ts:181-191
Timestamp: 2025-08-14T14:42:54.411Z
Learning: The countryCodeMap in src/components/AddMoney/consts/index.ts uses uppercase 3-letter country codes as keys (like 'AUT', 'BEL', 'CZE') that map to 2-letter country codes, requiring input normalization to uppercase for proper lookups.
Applied to files:
src/components/AddWithdraw/AddWithdrawCountriesList.tsx
📚 Learning: 2025-08-13T18:22:01.941Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1094
File: src/components/AddWithdraw/DynamicBankAccountForm.tsx:0-0
Timestamp: 2025-08-13T18:22:01.941Z
Learning: In the DynamicBankAccountForm component, the countryName parameter from useParams will always resemble a country title, not a URL slug.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
🧬 Code graph analysis (5)
src/redux/types/bank-form.types.ts (1)
src/components/AddWithdraw/DynamicBankAccountForm.tsx (1)
IBankAccountDetails(24-39)
src/components/Claim/Link/views/BankFlowManager.view.tsx (2)
src/redux/hooks.ts (1)
useAppDispatch(5-5)src/redux/slices/bank-form-slice.ts (1)
bankFormActions(36-36)
src/components/AddWithdraw/AddWithdrawCountriesList.tsx (2)
src/redux/hooks.ts (1)
useAppDispatch(5-5)src/redux/slices/bank-form-slice.ts (1)
bankFormActions(36-36)
src/components/AddWithdraw/DynamicBankAccountForm.tsx (2)
src/redux/hooks.ts (2)
useAppDispatch(5-5)useAppSelector(6-6)src/redux/slices/bank-form-slice.ts (1)
bankFormActions(36-36)
src/redux/slices/bank-form-slice.ts (3)
src/redux/types/bank-form.types.ts (1)
IBankFormState(5-7)src/redux/constants/index.ts (1)
BANK_FORM_SLICE(7-7)src/components/AddWithdraw/DynamicBankAccountForm.tsx (1)
IBankAccountDetails(24-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (9)
src/redux/constants/index.ts (1)
7-7: LGTM: Consistent slice name constant.Name follows existing pattern and is distinct. No issues.
src/redux/store.ts (1)
8-8: Reducer wiring and selectors verified –bankFormis registered inconfigureStore, included inRootState(src/redux/types/index.ts), and referenced in selectors (e.g.DynamicBankAccountForm.tsx:77).src/components/AddWithdraw/AddWithdrawCountriesList.tsx (3)
32-34: LGTM: Typed Redux hooks/actions import.Imports are correct and scoped.
47-47: LGTM: Single dispatch instance.Good placement; avoids re-creating in handlers.
236-238: Right call: Clear persisted form data on back from form.Prevents stale prefill when returning to list. Matches PR intent.
src/components/Claim/Link/views/BankFlowManager.view.tsx (2)
31-33: LGTM: Dispatch wiring for form-state management.Imports and
useAppDispatch()initialization are correct.Also applies to: 70-70
443-449: Clear form state before step transition—good safeguard.Ensures
DynamicBankAccountFormmounts clean without stale data.src/components/AddWithdraw/DynamicBankAccountForm.tsx (2)
17-18: Redux hooks/actions import looks correct.Imports align with store wiring and action export.
65-65: Dispatch instance initialized appropriately.No issues.
There was a problem hiding this comment.
@Zishan-7 approving for fast tracking, lgtm, but why use redux? context not working?
also check codderrabbit comments if actionable, and resolve merge conflicts
…sist-dynamicBankAccountFormState
I thought there are already a lot of states in |
…sist-dynamicBankAccountFormState
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/AddWithdraw/DynamicBankAccountForm.tsx (2)
82-84: Guard persisted defaults by country (and flow) to avoid cross-country/flow prefill.If a user switches countries or flows, stale values may prefill invalid fields. Gate application of persisted defaults.
Apply this minimal guard:
- ...persistedFormData, // Redux persisted data takes precedence + ...(persistedFormData && persistedFormData.country === country + ? persistedFormData + : {}), // only preload when matching countryOptionally, also gate by flow if tracked in the slice:
- ...(persistedFormData && persistedFormData.country === country - ? persistedFormData - : {}), + ...(persistedFormData + && persistedFormData.country === country + && (('flow' in persistedFormData ? (persistedFormData as any).flow === flow : true)) + ? persistedFormData + : {}),Also applies to: 106-106
201-211: Deduplicate spinner toggles; rely on a single place.You set isSubmitting(false) in both branches and again in finally. Prefer one location for clarity and to avoid double state churn.
- setIsSubmitting(false) } else { // Save form data to Redux after successful submission const formDataToSave = { ...data, country, firstName: data.firstName.trim(), lastName: data.lastName.trim(), } dispatch(bankFormActions.setFormData(formDataToSave)) - setIsSubmitting(false)Keep the finally block as the single source of truth.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/components/AddWithdraw/DynamicBankAccountForm.tsx(5 hunks)src/components/Claim/Link/views/BankFlowManager.view.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Claim/Link/views/BankFlowManager.view.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-22T15:38:48.586Z
Learnt from: kushagrasarathe
PR: peanutprotocol/peanut-ui#869
File: src/app/(mobile-ui)/withdraw/page.tsx:82-88
Timestamp: 2025-05-22T15:38:48.586Z
Learning: The country-specific withdrawal route exists at src/app/(mobile-ui)/withdraw/[...country]/page.tsx and renders the AddWithdrawCountriesList component with flow="withdraw".
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2025-08-13T18:22:01.941Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1094
File: src/components/AddWithdraw/DynamicBankAccountForm.tsx:0-0
Timestamp: 2025-08-13T18:22:01.941Z
Learning: In the DynamicBankAccountForm component, the countryName parameter from useParams will always resemble a country title, not a URL slug.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
🧬 Code graph analysis (1)
src/components/AddWithdraw/DynamicBankAccountForm.tsx (2)
src/redux/hooks.ts (2)
useAppDispatch(5-5)useAppSelector(6-6)src/redux/slices/bank-form-slice.ts (1)
bankFormActions(36-36)
🔇 Additional comments (1)
src/components/AddWithdraw/DynamicBankAccountForm.tsx (1)
17-18: Clarify Redux usage forbankFormslice and persistence strategy
ThebankFormreducer is registered in src/redux/store.ts but no redux-persist configuration was found—state remains in-memory and is lost on refresh. Confirm this aligns with requirements, or if the form state only needs to live within the Add/Withdraw flow, consider using a flow-scoped context (e.g. WithdrawFlowContext) instead of Redux.
No description provided.