Conversation
- Replace addRecipient function with verifySenderBankOtp to streamline the OTP verification process. - Introduce otpRefId state to manage OTP reference ID for verification. - Adjust loading state to reflect the verification process accurately.
- Add a description to the error toast for failed transactions to provide users with more context on the failure. This enhances user experience by guiding them on potential next steps.
- Added the 'required' attribute to the amount input label in the LoadWalletStep component. - This change ensures that users are aware that entering an amount is mandatory for wallet loading.
- Update the success flow for OTP verification to provide user feedback through a toast notification and navigate to the wallet dashboard. - Removed unused code related to beneficiary ID handling to streamline the process and improve code clarity.
Fix/digikhata
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the recipient addition and verification process by integrating the verifySenderBankOtp API and managing otp_ref_id state. It also enhances error reporting in the fund transfer step and adds a required attribute to the amount input in the wallet loading step. The review feedback highlights opportunities to improve type safety by initializing state with empty strings instead of null and using nullish coalescing. Furthermore, there are large blocks of commented-out code in AddRecipientStep.tsx and RecipientsStep.tsx that should be cleaned up, and the shift in user navigation from the fund transfer step to the wallet dashboard needs verification to ensure it aligns with the intended user experience.
| // const recipientData = res.data.data?.recipient; | ||
| // const newRecipient = { | ||
| // recipient_id: recipientData?.recipient_id ?? 0, | ||
| // bank_recipient_id: recipientData?.bank_recipient_id ?? null, | ||
| // name: recipientData?.recipient_name ?? recipientName.trim(), | ||
| // accountNumber: accountNumber.trim(), | ||
| // ifsc: ifsc.trim().toUpperCase(), | ||
| // bankName: selectedBank!.label, | ||
| // accountType: "Bank Account", | ||
| // isVerified: recipientData?.is_verified === 1, | ||
| // mobile: recipientMobile.trim(), | ||
| // recipientIdType: "acc_ifsc", | ||
| // beneficiary_id: res.data.data?.beneficiary_id ?? null, | ||
| // isNew: true, | ||
| // }; | ||
| // dispatch({ | ||
| // type: "ADD_RECIPIENT", | ||
| // payload: newRecipient, | ||
| // }); |
There was a problem hiding this comment.
This block of code is commented out. If the logic for updating the local state is no longer required, please remove it. Otherwise, restore it to ensure the UI remains in sync without needing a full list refresh after adding a recipient.
References
- Commented-out code should be removed to maintain code clarity and reduce technical debt.
| dispatch({ type: "SET_STEP", step: "wallet-dashboard" }); | ||
| // const beneficiaryId = res.data.data?.beneficiary_id; | ||
| // if (beneficiaryId) { | ||
| // // Update global context so the recipient is fixed | ||
| // dispatch({ | ||
| // type: "UPDATE_RECIPIENT_BENEFICIARY", | ||
| // payload: { | ||
| // recipient_id: pendingRecipient.recipient_id, | ||
| // beneficiary_id: beneficiaryId, | ||
| // }, | ||
| // }); | ||
|
|
||
| // Set selected and move forward | ||
| const updatedRecipient = { | ||
| ...pendingRecipient, | ||
| beneficiary_id: beneficiaryId, | ||
| }; | ||
| dispatch({ | ||
| type: "SET_SELECTED_RECIPIENT", | ||
| payload: updatedRecipient, | ||
| }); | ||
| setIsOtpOpen(false); | ||
| dispatch({ type: "SET_STEP", step: "fund-transfer" }); | ||
| } else { | ||
| toast({ | ||
| title: "Failed to fetch updated beneficiary details", | ||
| status: "error", | ||
| duration: 4000, | ||
| isClosable: true, | ||
| }); | ||
| } | ||
| // // Set selected and move forward | ||
| // const updatedRecipient = { | ||
| // ...pendingRecipient, | ||
| // beneficiary_id: beneficiaryId, | ||
| // }; | ||
| // dispatch({ | ||
| // type: "SET_SELECTED_RECIPIENT", | ||
| // payload: updatedRecipient, | ||
| // }); | ||
| // setIsOtpOpen(false); | ||
| // dispatch({ type: "SET_STEP", step: "fund-transfer" }); | ||
| // } else { | ||
| // toast({ | ||
| // title: "Failed to fetch updated beneficiary details", | ||
| // status: "error", | ||
| // duration: 4000, | ||
| // isClosable: true, | ||
| // }); | ||
| // } |
There was a problem hiding this comment.
The logic for updating the recipient and navigating to the transfer step is commented out. Additionally, the redirection to wallet-dashboard on line 130 changes the user flow from the previous implementation. Please clean up the dead code and verify if the navigation should still point to fund-transfer after successful verification to maintain a smooth user experience.
References
- Commented-out code should be removed to maintain code clarity and reduce technical debt.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
2 participants
Description
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
🚨 Checklist:
Further comments
🙏 Thank you!
Thank you for contributing to this project. We appreciate your time and effort. 🎉