-
Notifications
You must be signed in to change notification settings - Fork 47
feat: Update warning styles and add recipient accuracy reminder in forms #231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: Update warning styles and add recipient accuracy reminder in forms #231
Conversation
📝 Walkthrough🚥 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)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/components/recipient/RecipientDetailsForm.tsx(9 hunks)app/pages/TransactionForm.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/components/recipient/RecipientDetailsForm.tsx (3)
app/components/AnimatedComponents.tsx (1)
AnimatedFeedbackItem(275-288)app/utils.ts (1)
classNames(22-24)app/components/InputError.tsx (1)
InputError(5-20)
…ecipient name editability
… recipient name verification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/components/recipient/RecipientDetailsForm.tsx(9 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T16:44:32.125Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 244
File: app/components/CopyAddressWarningModal.tsx:48-52
Timestamp: 2025-10-10T16:44:32.125Z
Learning: In the CopyAddressWarningModal component (app/components/CopyAddressWarningModal.tsx), selectedNetwork from useNetwork() is always defined and does not require null safety checks when accessing its properties like selectedNetwork.chain.name.
Applied to files:
app/components/recipient/RecipientDetailsForm.tsx
🧬 Code graph analysis (1)
app/components/recipient/RecipientDetailsForm.tsx (3)
app/components/AnimatedComponents.tsx (1)
AnimatedFeedbackItem(275-288)app/utils.ts (1)
classNames(22-24)app/components/InputError.tsx (1)
InputError(5-20)
🔇 Additional comments (7)
app/components/recipient/RecipientDetailsForm.tsx (7)
5-5: LGTM!The
InformationSquareIconimport is correctly added and used in the warning banners below.
61-61: LGTM!The state declaration for tracking editable recipient name is correct and properly initialized.
128-128: LGTM!Correctly disables manual entry mode when selecting a saved recipient with a pre-verified name.
243-243: LGTM!Correctly resets the editable state when the institution changes, ensuring clean state for new verification.
283-284: LGTM!Correctly disables manual entry and displays an error message when account name verification fails.
332-332: LGTM!Correctly resets the editable state when clearing all recipient details.
434-459: LGTM!The conditional rendering for editable vs verified recipient name is well-implemented with proper validation and error handling.
…recipient name based on error state
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/components/recipient/RecipientDetailsForm.tsx (1)
494-511: Alert condition is incomplete and misses validation error state.The alert conditions on lines 495 and 503 don't check for form validation errors (
errors.recipientName) or the fetching state. Per the PR objectives: "Alert is shown only when recipientName is present and there is no recipientNameError; validation/error state takes precedence."When a user enters an invalid name (e.g., 1 character),
recipientNamewill contain that value ANDerrors.recipientNamewill be set, yet the alert will still render. The previous review comment identified this exact issue.Apply this diff to fix the conditions:
- <AnimatedFeedbackItem> - {isRecipientNameEditable && recipientName && !recipientNameError && ( + {recipientName && !recipientNameError && !errors.recipientName && !isFetchingRecipientName && ( + <AnimatedFeedbackItem> + {isRecipientNameEditable ? ( <div className="min-h-[48px] h-fit w-full dark:bg-warning-background/10 bg-warning-background/35 px-3 py-2 rounded-xl flex items-start gap-2"> <InformationSquareIcon className="dark:text-warning-text text-warning-foreground w-[36px] h-[36px] md:w-[24px] md:h-[24px]" /> <p className="text-xs font-light dark:text-warning-text text-warning-foreground leading-tight"> Unable to verify details. Ensure the recipient's account number is accurate before proceeding with swap. <a href="#" className="text-lavender-500 text-semibold">Learn more.</a> </p> </div> - )} - {!isRecipientNameEditable && recipientName && !recipientNameError && ( + ) : ( <div className="min-h-[48px] h-fit w-full dark:bg-warning-background/10 bg-warning-background/35 px-3 py-2 rounded-xl flex items-start gap-2"> <InformationSquareIcon className="dark:text-warning-text text-warning-foreground w-[36px] h-[36px] md:w-[24px] md:h-[24px]" /> <p className="text-xs font-light dark:text-warning-text text-warning-foreground leading-tight"> Make sure the recipient's account number is accurate before proceeding with swap. <a href="#" className="text-lavender-500 text-semibold">Learn more.</a> </p> </div> - )} - </AnimatedFeedbackItem> + )} + </AnimatedFeedbackItem> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/components/recipient/RecipientDetailsForm.tsx(9 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:271-280
Timestamp: 2025-11-06T07:08:54.567Z
Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.
📚 Learning: 2025-11-06T07:08:54.567Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:271-280
Timestamp: 2025-11-06T07:08:54.567Z
Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.
Applied to files:
app/components/recipient/RecipientDetailsForm.tsx
🧬 Code graph analysis (1)
app/components/recipient/RecipientDetailsForm.tsx (3)
app/components/AnimatedComponents.tsx (1)
AnimatedFeedbackItem(275-288)app/utils.ts (1)
classNames(22-24)app/components/InputError.tsx (1)
InputError(5-20)
🔇 Additional comments (9)
app/components/recipient/RecipientDetailsForm.tsx (9)
5-5: LGTM!Import addition is appropriate for the new warning alerts below.
61-61: LGTM!State declaration is appropriate for managing the editable/non-editable recipient name flow.
128-128: LGTM!Correctly resets editable state when selecting a saved recipient, ensuring the verified name is displayed read-only.
243-243: LGTM!Appropriately resets editable state when institution changes during manual entry flow.
284-284: LGTM!Correctly disables editable mode when verification encounters an error.
332-332: LGTM!Properly resets editable state when clearing recipient details.
271-280: LGTM!The verification logic correctly handles the "Ok" response for manual entry fallback. Based on learnings, Mobile Money institutions always return "Ok" from verification, so this check is sufficient without additional institution type checks.
434-459: LGTM!The editable input implementation correctly renders when manual entry is required, includes appropriate validation rules, and displays errors properly.
499-507: Fix text accuracy and styling issues.Multiple small issues with the alert text and styling:
- Line 507: Missing "your" before "swap" — requirements specify "your swap"
- Lines 499, 507: Missing period before "Learn more"
- Lines 499, 507:
text-semiboldshould befont-semibold(correct Tailwind class)Apply this diff:
- Unable to verify details. Ensure the recipient's account number is accurate before proceeding with swap. <a href="#" className="text-lavender-500 text-semibold">Learn more.</a> + Unable to verify details. Ensure the recipient's account number is accurate before proceeding with your swap. <a href="#" className="text-lavender-500 font-semibold">Learn more.</a>- Make sure the recipient's account number is accurate before proceeding with swap. <a href="#" className="text-lavender-500 text-semibold">Learn more.</a> + Make sure the recipient's account number is accurate before proceeding with your swap. <a href="#" className="text-lavender-500 font-semibold">Learn more.</a>⛔ Skipped due to learnings
Learnt from: Dprof-in-tech Repo: paycrest/noblocks PR: 231 File: app/components/recipient/RecipientDetailsForm.tsx:271-280 Timestamp: 2025-11-06T07:08:54.567Z Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.
…recipient name to include form validation errors
…rification feedback and track alert visibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/components/recipient/RecipientDetailsForm.tsx(13 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:494-511
Timestamp: 2025-11-06T07:25:06.205Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), the recipient safety alert should only display when all of the following conditions are met: recipientName is present, !recipientNameError (no API error), !errors.recipientName (no form validation errors), and !isFetchingRecipientName. This ensures validation errors take precedence over the alert as per PR #231 requirements.
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:271-280
Timestamp: 2025-11-06T07:08:54.567Z
Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.
📚 Learning: 2025-11-06T07:25:06.205Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:494-511
Timestamp: 2025-11-06T07:25:06.205Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), the recipient safety alert should only display when all of the following conditions are met: recipientName is present, !recipientNameError (no API error), !errors.recipientName (no form validation errors), and !isFetchingRecipientName. This ensures validation errors take precedence over the alert as per PR #231 requirements.
Applied to files:
app/components/recipient/RecipientDetailsForm.tsx
📚 Learning: 2025-11-06T07:08:54.567Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:271-280
Timestamp: 2025-11-06T07:08:54.567Z
Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.
Applied to files:
app/components/recipient/RecipientDetailsForm.tsx
🧬 Code graph analysis (1)
app/components/recipient/RecipientDetailsForm.tsx (3)
app/components/AnimatedComponents.tsx (1)
AnimatedFeedbackItem(275-288)app/utils.ts (1)
classNames(22-24)app/components/InputError.tsx (1)
InputError(5-20)
…ipient name and error state
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/components/recipient/RecipientDetailsForm.tsx(13 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.019Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is true (verification failed/returned "Ok"), the recipient safety alert should display when: isRecipientNameEditable && recipientName && !errors.recipientName && !recipientNameError. The !isFetchingRecipientName check is redundant because recipientName is cleared at fetch start and only populated after fetching completes or when the user manually enters it.
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.019Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is false (verification succeeded), the recipient safety alert should display when: !isRecipientNameEditable && recipientName && !recipientNameError. The !errors.recipientName check is unnecessary because in non-editable mode the recipient name is displayed as read-only text (not an input field), so form validation errors cannot occur.
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:271-280
Timestamp: 2025-11-06T07:08:54.567Z
Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.
📚 Learning: 2025-11-06T07:37:39.019Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.019Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is true (verification failed/returned "Ok"), the recipient safety alert should display when: isRecipientNameEditable && recipientName && !errors.recipientName && !recipientNameError. The !isFetchingRecipientName check is redundant because recipientName is cleared at fetch start and only populated after fetching completes or when the user manually enters it.
Applied to files:
app/components/recipient/RecipientDetailsForm.tsx
📚 Learning: 2025-11-06T07:37:39.019Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.019Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is false (verification succeeded), the recipient safety alert should display when: !isRecipientNameEditable && recipientName && !recipientNameError. The !errors.recipientName check is unnecessary because in non-editable mode the recipient name is displayed as read-only text (not an input field), so form validation errors cannot occur.
Applied to files:
app/components/recipient/RecipientDetailsForm.tsx
📚 Learning: 2025-11-06T07:08:54.567Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:271-280
Timestamp: 2025-11-06T07:08:54.567Z
Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.
Applied to files:
app/components/recipient/RecipientDetailsForm.tsx
🧬 Code graph analysis (1)
app/components/recipient/RecipientDetailsForm.tsx (3)
app/components/AnimatedComponents.tsx (1)
AnimatedFeedbackItem(275-288)app/utils.ts (1)
classNames(22-24)app/components/InputError.tsx (1)
InputError(5-20)
* feat: complete glitchtip integration with sentry wizard * fix: add glitchtip url to env * fix: resolve CodeRabbit review – harden PII handling, env example & beforeSend parsing * fix: harden beforeSend fallback * fix: add base64url-safe JWT decoding in beforeSend for reliable Privy session extraction * fix: update glitchtip public-url * fix: add release to withSentryConfig * refactor: migrate from GlitchTip to Sentry for error tracking - Updated environment variables in .env.example to reflect Sentry configuration. - Refactored Sentry initialization in instrumentation-client.ts, sentry.edge.config.ts, and sentry.server.config.ts to use Sentry DSN and environment variables. - Adjusted config types and values in app/lib/config.ts to support Sentry integration. - Updated package dependencies to use the latest version of @sentry/nextjs and related packages. - Ensured proper handling of PII and traces sample rate based on the environment.
* feat: enhance user tracking with server-side event logging - Added server-side user identification and event tracking in various components including Navbar, TransactionPreview, and TransactionStatus. - Implemented `identifyServerUser` and `trackServerEvent` functions to log user actions such as login, logout, funding, and transaction events. - Updated `trackUserEvent` helper function to streamline user identification and event tracking. - Ensured backward compatibility with existing client-side tracking methods. * feat: enhance analytics tracking with IP and User-Agent logging - Updated the analytics identify and track routes to include IP address and User-Agent in tracking requests. - Refactored event tracking functions to utilize common properties for improved consistency and clarity. - Ensured that sensitive data is sanitized before being sent to analytics services for geo-inference and device detection. * refactor: streamline server event tracking and IP inclusion logic - Removed redundant trackServerEvent call in analytics tracking. - Updated trackServerEvent invocation to standardize the event name format. - Enhanced IP address inclusion logic to respect privacy settings, ensuring compliance with user privacy preferences. * refactor: remove trackUserEvent function * refactor: improve wallet address handling and event tracking logic - Updated wallet address retrieval to use undefined instead of null for better type handling. - Streamlined event tracking by ensuring wallet address is only included when present, allowing for generic event tracking when absent. - Enhanced the handling of transaction, funding, and authentication events to ensure they are tracked appropriately based on wallet address availability. - Removed redundant server-side tracking calls in favor of a more consistent client-side approach. * refactor: remove wallet address retrieval for server-side tracking * refactor: standardize properties handling in event tracking functions - Updated event tracking functions to use default empty object for properties parameter, improving consistency and reducing the need for undefined checks. - This change applies to multiple tracking functions including trackPageView, trackBlogCardClick, and trackSearch. * Update app/hooks/analytics/useMixpanel.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Chibuotu Amadi <chibuotu@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Removed unused import of config and replaced it with environment variables for Sentry URL and auth token. - Standardized formatting in serverExternalPackages and webpack configuration for consistency. - Ensured proper structure in the rules section of the webpack configuration. - Minor adjustments to maintain code clarity and organization.
…fig.mjs - Introduced a new configuration option to disable source map uploads in low-memory environments. - Added a script in package.json for building in low-memory mode, setting appropriate Node options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/components/recipient/RecipientDetailsForm.tsx (1)
87-107: Consider extracting RecipientAlert for reusability.The component is well-implemented and correctly handles analytics. Defining it inside the parent function means it's recreated on every render, though React handles this efficiently.
If you prefer, you could extract it outside and pass
currencyandselectedInstitution?.nameas props, but the current approach is acceptable given the tight coupling with parent state.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/components/recipient/RecipientDetailsForm.tsx(13 hunks)app/pages/TransactionForm.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/pages/TransactionForm.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is true (verification failed/returned "Ok"), the recipient safety alert should display when: isRecipientNameEditable && recipientName && !errors.recipientName && !recipientNameError. The !isFetchingRecipientName check is redundant because recipientName is cleared at fetch start and only populated after fetching completes or when the user manually enters it.
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is false (verification succeeded), the recipient safety alert should display when: !isRecipientNameEditable && recipientName && !recipientNameError. The !errors.recipientName check is unnecessary because in non-editable mode the recipient name is displayed as read-only text (not an input field), so form validation errors cannot occur.
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:271-280
Timestamp: 2025-11-06T07:08:54.593Z
Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is true (verification failed/returned "Ok"), the recipient safety alert should display when: isRecipientNameEditable && recipientName && !errors.recipientName && !recipientNameError. The !isFetchingRecipientName check is redundant because recipientName is cleared at fetch start and only populated after fetching completes or when the user manually enters it.
Applied to files:
app/components/recipient/RecipientDetailsForm.tsx
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is false (verification succeeded), the recipient safety alert should display when: !isRecipientNameEditable && recipientName && !recipientNameError. The !errors.recipientName check is unnecessary because in non-editable mode the recipient name is displayed as read-only text (not an input field), so form validation errors cannot occur.
Applied to files:
app/components/recipient/RecipientDetailsForm.tsx
📚 Learning: 2025-11-06T07:08:54.593Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:271-280
Timestamp: 2025-11-06T07:08:54.593Z
Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.
Applied to files:
app/components/recipient/RecipientDetailsForm.tsx
🧬 Code graph analysis (1)
app/components/recipient/RecipientDetailsForm.tsx (3)
app/components/AnimatedComponents.tsx (1)
AnimatedFeedbackItem(275-288)app/utils.ts (1)
classNames(23-25)app/components/InputError.tsx (1)
InputError(5-20)
⏰ 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: build (20.x)
🔇 Additional comments (6)
app/components/recipient/RecipientDetailsForm.tsx (6)
5-5: LGTM!The new imports (
InformationSquareIconandtrackEvent) are correctly used for rendering the alert UI and tracking analytics events.Also applies to: 14-14
62-62: LGTM!The new state variables correctly manage the editable name flow and per-instance analytics tracking.
Also applies to: 73-73
216-235: LGTM!The analytics tracking correctly matches the actual alert visibility conditions per the learnings. The
shouldShowAlertlogic properly handles both editable and non-editable cases, and the dependency array is complete.Based on learnings.
318-331: LGTM!The verification logic correctly handles the "Ok" response by enabling manual entry mode. Per the learnings, Mobile Money institutions always return "Ok", so this check appropriately covers both verification failures and Mobile Money without needing explicit institution type checks.
Based on learnings.
481-506: LGTM!The editable recipient name input is correctly implemented with appropriate validation (required, minimum 2 characters) and error handling. The integration with react-hook-form and conditional styling work well.
541-554: LGTM!The alert rendering conditions correctly match the learnings:
- Editable case (line 542): Checks all four conditions including form validation errors
- Non-editable case (line 548): Omits
errors.recipientNamecheck since the name is read-onlyThe conditions ensure alerts display only when appropriate, with validation errors taking precedence.
Based on learnings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/components/recipient/RecipientDetailsForm.tsx (1)
90-127: Update anchor href to use the actual URL for better accessibility.The "Learn more" link has a real URL defined at line 108 and opens it via JavaScript, but the
hrefattribute still uses"#"as a placeholder. This creates accessibility and UX issues:
- Screen readers won't announce the correct destination
- Users can't right-click to copy/open the link
- Keyboard navigation shows "#" in the status bar
- Link won't work if JavaScript is disabled
🔎 Suggested improvement
Update the anchor tag to use the actual URL and add security attributes:
<a - href="#" + href="https://noblocks.xyz/blog/understanding-account-name-verification-on-noblocks" + target="_blank" + rel="noopener noreferrer" onClick={handleLearnMoreClick} className="font-semibold text-lavender-500" > Learn more. </a>Then update the click handler to prevent default navigation since you're tracking analytics:
const handleLearnMoreClick = (e: React.MouseEvent) => { - e.preventDefault(); trackEvent("recipient_alert_learn_more_clicked", { alert_type: isEditable ? "verification_failed" : "verification_success", message: message.substring(0, 100), currency: currency, institution: selectedInstitution?.name || "", }); - // Open relevant help article in new tab - const helpUrl = - "https://noblocks.xyz/blog/understanding-account-name-verification-on-noblocks"; - window.open(helpUrl, "_blank"); };This approach:
- Maintains the analytics tracking
- Provides proper accessibility
- Works without JavaScript
- Follows security best practices with
rel="noopener noreferrer"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/components/recipient/RecipientDetailsForm.tsx(12 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is true (verification failed/returned "Ok"), the recipient safety alert should display when: isRecipientNameEditable && recipientName && !errors.recipientName && !recipientNameError. The !isFetchingRecipientName check is redundant because recipientName is cleared at fetch start and only populated after fetching completes or when the user manually enters it.
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is false (verification succeeded), the recipient safety alert should display when: !isRecipientNameEditable && recipientName && !recipientNameError. The !errors.recipientName check is unnecessary because in non-editable mode the recipient name is displayed as read-only text (not an input field), so form validation errors cannot occur.
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:271-280
Timestamp: 2025-11-06T07:08:54.593Z
Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is true (verification failed/returned "Ok"), the recipient safety alert should display when: isRecipientNameEditable && recipientName && !errors.recipientName && !recipientNameError. The !isFetchingRecipientName check is redundant because recipientName is cleared at fetch start and only populated after fetching completes or when the user manually enters it.
Applied to files:
app/components/recipient/RecipientDetailsForm.tsx
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is false (verification succeeded), the recipient safety alert should display when: !isRecipientNameEditable && recipientName && !recipientNameError. The !errors.recipientName check is unnecessary because in non-editable mode the recipient name is displayed as read-only text (not an input field), so form validation errors cannot occur.
Applied to files:
app/components/recipient/RecipientDetailsForm.tsx
📚 Learning: 2025-11-06T07:08:54.593Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:271-280
Timestamp: 2025-11-06T07:08:54.593Z
Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.
Applied to files:
app/components/recipient/RecipientDetailsForm.tsx
🧬 Code graph analysis (1)
app/components/recipient/RecipientDetailsForm.tsx (3)
app/components/AnimatedComponents.tsx (1)
AnimatedFeedbackItem(275-288)app/utils.ts (1)
classNames(23-25)app/components/InputError.tsx (1)
InputError(5-20)
⏰ 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: build (20.x)
🔇 Additional comments (6)
app/components/recipient/RecipientDetailsForm.tsx (6)
5-9: LGTM!The new imports for
InformationSquareIconandtrackEventare properly integrated and used throughout the component.Also applies to: 18-18
236-268: LGTM!The analytics tracking logic correctly aligns with alert visibility conditions and includes all necessary dependencies. The implementation properly handles both editable and non-editable cases per the established requirements.
Based on learnings.
352-366: LGTM!The verification logic correctly handles the "Ok" response by enabling manual name entry. The implementation aligns with the backend behavior where Mobile Money institutions and failed verifications return "Ok", making explicit institution type checks unnecessary.
Based on learnings.
514-544: LGTM!The editable recipient name input is properly implemented with appropriate validation rules, error handling, and styling. The form integration follows React Hook Form best practices.
578-594: LGTM!The conditional alert rendering correctly implements the visibility logic per the established requirements. The conditions properly distinguish between editable (verification failed) and non-editable (verification succeeded) cases, ensuring alerts display only when appropriate.
Based on learnings.
174-174: LGTM!The
isRecipientNameEditablestate is properly reset in all relevant flows (selecting saved recipient, changing institution, clearing details), ensuring correct behavior throughout the component lifecycle.Also applies to: 323-323, 412-412
06dafbf to
5b287b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/types.ts`:
- Around line 263-266: The Config type currently exposes server-only fields;
remove sentryUrl and sentryAuthToken from the exported Config type and move
Sentry-specific values out of the shared client/server config: delete the
sentryUrl and sentryAuthToken properties from the Config interface (symbol:
Config) and create a server-only configuration (or read directly from
process.env) used only by sentry.server.config.ts and sentry.edge.config.ts to
access sentryDsn and nodeEnv; ensure client code only imports the pared-down
Config (containing no server secrets) and update any imports referencing
sentryUrl/sentryAuthToken to use the new server-side config or process.env.
🧹 Nitpick comments (1)
package.json (1)
57-57: These are likely Sentry's transitive dependencies.
import-in-the-middleandrequire-in-the-middleare instrumentation libraries used by APM tools like Sentry for auto-instrumentation. Adding them explicitly is typically done to resolve version conflicts or hoisting issues.If these were added to fix a specific build or runtime error, consider adding a comment in the package.json or a note in the PR description explaining why they're needed as direct dependencies.
Also applies to: 76-76
| sentryDsn: string; | ||
| nodeEnv: string; | ||
| sentryUrl: string; | ||
| sentryAuthToken: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the Config type definition
echo "=== Config type definition ==="
sed -n '250,280p' app/types.ts
# Find all references to Config type
echo -e "\n=== Config type usage ==="
rg "Config" app/ --type=ts --type=tsx -n | head -30
# Check for client-side exposure patterns
echo -e "\n=== SentryAuthToken usage ==="
rg "sentryAuthToken|SENTRY_AUTH_TOKEN" . --type=ts --type=tsx -C2
# Check for config serialization to client
echo -e "\n=== Config export/serialization patterns ==="
rg "export.*config|getConfig|publicConfig" app/ --type=ts --type=tsx -nRepository: paycrest/noblocks
Length of output: 1013
🏁 Script executed:
#!/bin/bash
# Find where Config is instantiated/created
echo "=== Config instantiation ==="
rg -n "Config\s*=" app/ --type ts | head -20
# Look for function that returns Config
echo -e "\n=== Functions returning Config ==="
rg -n ":\s*Config\s*[={]" app/ --type ts -B2 | head -30
# Check for config object/variable assignments
echo -e "\n=== Config variable definitions ==="
rg -n "const.*config|export.*config" app/ --type ts -i | head -30
# Search for where sentryAuthToken is actually used
echo -e "\n=== sentryAuthToken usage ==="
rg -n "sentryAuthToken" . --type ts -C3
# Check if config is passed to response or exposed to client
echo -e "\n=== Config in responses/context ==="
rg -n "sendConfig|res\..*config|context.*config" app/ --type ts -i | head -20Repository: paycrest/noblocks
Length of output: 3667
🏁 Script executed:
#!/bin/bash
# Read the config file to see clientConfig vs serverConfig separation
echo "=== Full config.ts file ==="
cat -n app/lib/config.tsRepository: paycrest/noblocks
Length of output: 3003
🏁 Script executed:
#!/bin/bash
# Check where the default config is imported
echo "=== Default config imports ==="
rg "from.*['\"]@/app/lib/config['\"]|import.*config.*from" app/ --type ts -B1 -A1 | head -40
# Check Sentry initialization code
echo -e "\n=== Sentry initialization ==="
rg -n "sentry|Sentry" app/ --type ts | head -30
# Check if config is used in client components
echo -e "\n=== Config in client components (TSX) ==="
find app/ -name "*.tsx" -type f | head -20
# Check for getServerSideProps/getStaticProps patterns
echo -e "\n=== API routes that might expose config ==="
find app/api -name "*.ts" -type f | head -20Repository: paycrest/noblocks
Length of output: 3744
🏁 Script executed:
#!/bin/bash
# Check how config is actually used in client components
echo "=== Config usage in providers.tsx ==="
sed -n '1,50p' app/providers.tsx
echo -e "\n=== Config usage in AppLayout.tsx ==="
rg -n "config" app/components/AppLayout.tsx -C2 | head -30
echo -e "\n=== Check if config is passed to context or props ==="
rg -n "config\." app/providers.tsx -C2
# Check Sentry initialization usage
echo -e "\n=== Sentry configuration ==="
find . -name "sentry*" -type f 2>/dev/null | grep -v node_modulesRepository: paycrest/noblocks
Length of output: 2801
🏁 Script executed:
#!/bin/bash
# Check Sentry configuration files
echo "=== sentry.server.config.ts ==="
cat -n sentry.server.config.ts
echo -e "\n=== sentry.edge.config.ts ==="
cat -n sentry.edge.config.ts
echo -e "\n=== Check if sentryAuthToken/sentryUrl/nodeEnv are used in Sentry config ==="
rg -n "sentryAuthToken|sentryUrl|nodeEnv" sentry* --type ts
# Check if full config object is ever serialized/exposed
echo -e "\n=== Check if config is serialized or stringified ==="
rg -n "JSON\.stringify.*config|JSON\.parse.*config|export.*config" app/ --type tsRepository: paycrest/noblocks
Length of output: 1845
Remove unused sentryUrl and sentryAuthToken from the Config type; extract Sentry-only fields to server-side configuration.
sentryUrl and sentryAuthToken are defined in the Config type but never actually used in the codebase. While Next.js protects non-NEXT_PUBLIC_ environment variables from being exposed to the browser, including server-only secrets in a shared Config type that's imported in client code is poor architectural practice and creates maintenance risks.
Additionally, sentryDsn and nodeEnv are only used for Sentry initialization in sentry.server.config.ts and sentry.edge.config.ts. These should be extracted to a separate server-side configuration file rather than being part of the general Config type imported by client components.
🤖 Prompt for AI Agents
In `@app/types.ts` around lines 263 - 266, The Config type currently exposes
server-only fields; remove sentryUrl and sentryAuthToken from the exported
Config type and move Sentry-specific values out of the shared client/server
config: delete the sentryUrl and sentryAuthToken properties from the Config
interface (symbol: Config) and create a server-only configuration (or read
directly from process.env) used only by sentry.server.config.ts and
sentry.edge.config.ts to access sentryDsn and nodeEnv; ensure client code only
imports the pared-down Config (containing no server secrets) and update any
imports referencing sentryUrl/sentryAuthToken to use the new server-side config
or process.env.
Description
This pull request enhances the recipient details form to better handle cases where recipient account verification fails, allowing users to manually enter the recipient name and providing clearer feedback. The main changes include introducing an editable recipient name input, updating error handling, and displaying a warning message when verification is unsuccessful.
Recipient Name Verification & Manual Entry
isRecipientNameEditableto control whether the recipient name field is editable by the user."Ok"(indicating verification failure but not an error), the form now enables manual entry of the recipient name and clears any previous value or error.User Interface & Feedback
Icon Usage
InformationSquareIconfor the warning message to visually highlight verification issues.References
closes #229
closes #275
Testing
Checklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes