-
Notifications
You must be signed in to change notification settings - Fork 13
Prod release fixes #844
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
Prod release fixes #844
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Caution Review failedThe pull request is closed. WalkthroughThe changes introduce automatic creation of a "peanut-wallet" account for users in the Home component, expand public path recognition in the mobile UI layout, update deposit link construction to use a configurable base URL, add a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HomeComponent
participant useWallet
participant useAuth
User->>HomeComponent: Loads Home page
HomeComponent->>useWallet: Get wallet address
HomeComponent->>useAuth: Get user info
HomeComponent->>HomeComponent: useEffect checks accounts
alt "peanut-wallet" account missing
HomeComponent->>useAuth: addAccount(address, "peanut-wallet", userId)
end
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 (3)
src/components/AddFunds/index.tsx (1)
214-214
: Consider handling potential undefined username.The new deposit link uses the centralized BASE_URL which is good, but there's no fallback if
user?.user?.username
is undefined.-const depositLink = `${BASE_URL}/pay/${user?.user?.username}` +const depositLink = user?.user?.username ? `${BASE_URL}/pay/${user.user.username}` : BASE_URLsrc/app/(mobile-ui)/home/page.tsx (1)
52-62
: Good implementation of automatic peanut-wallet account creation.This useEffect addresses the issue of users not having properly created peanut-wallet accounts. The dependencies array is correctly set to [user, address] to ensure the code runs when either changes.
Consider adding error handling to the account creation process:
useEffect(() => { // We have some users that didn't have the peanut wallet created // correctly, so we need to create it if (address && user && !user.accounts.some((a) => a.account_type === 'peanut-wallet')) { + try { addAccount({ accountIdentifier: address, accountType: 'peanut-wallet', userId: user.user.userId, }) + } catch (error) { + console.error('Failed to create peanut wallet account:', error) + } } }, [user, address])src/context/authContext.tsx (1)
95-134
: Provide user feedback for the “Add account” flow
addAccount
performs a network request but gives no visual cue on success or failure (except for thrown errors).
Consider adding optimistic UI or toast notifications similar to the logout flow so users know their new account was added.+ try { const response = await fetchWithSentry('/api/peanut/user/add-account', { /* … */ }) … - fetchUser() + fetchUser() + toast.success?.('Account added') + } catch (e) { + toast.error?.(e.message ?? 'Could not add account') + throw e + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/(mobile-ui)/home/page.tsx
(3 hunks)src/app/(mobile-ui)/layout.tsx
(1 hunks)src/components/AddFunds/index.tsx
(2 hunks)src/constants/general.consts.ts
(1 hunks)src/context/authContext.tsx
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/components/AddFunds/index.tsx (1)
src/constants/general.consts.ts (1)
BASE_URL
(36-36)
src/app/(mobile-ui)/home/page.tsx (2)
src/hooks/wallet/useWallet.ts (1)
useWallet
(20-82)src/context/authContext.tsx (1)
useAuth
(195-201)
src/context/authContext.tsx (1)
src/components/0_Bruddle/Toast.tsx (1)
useToast
(111-117)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (7)
src/constants/general.consts.ts (1)
36-36
: Good addition of a centralized BASE_URL constant.The addition of this configurable constant is a clean approach for centralizing URL construction across the application. It appropriately uses the environment variable with a sensible default fallback.
src/components/AddFunds/index.tsx (1)
13-13
: Good import of the BASE_URL constant.Properly importing the centralized constant from constants file.
src/app/(mobile-ui)/layout.tsx (1)
19-19
: Good extension of public path regex to support /pay/ URLs.The regex now correctly captures the new URL pattern for payment links, aligning with the updated deposit link format being used in the AddFunds component.
src/app/(mobile-ui)/home/page.tsx (2)
21-21
: Good addition of useEffect import.Properly importing the required hook for the new account creation functionality.
25-25
: Good destructuring of required dependencies.Correctly extracting the wallet address and the addAccount function needed for the automatic account creation.
Also applies to: 34-34
src/context/authContext.tsx (2)
9-10
: WrapAuthProvider
withToastProvider
to avoid runtime crashes
useToast
now comes from your custom Bruddle implementation which willthrow
if it is used outside of aToastProvider
(seeToast.tsx
, lines 111-117).
Double-check that every app entry-point whereAuthProvider
is rendered is already nested insideToastProvider
; otherwise navigation to any route that mountsAuthProvider
first will raise an exception and take the whole app down.Example (App Router –
layout.tsx
):-<AuthProvider> - {children} -</AuthProvider> +<ToastProvider> + <AuthProvider> + {children} + </AuthProvider> +</ToastProvider>Also applies to: 49-50
160-168
: Verify that the custom toast context exposessuccess
/error
helpersThe new calls
toast.success('Logged out successfully') toast.error('Failed to log out') toast.error('Error logging out')assume the context object has
success
anderror
methods (previously Chakra’suseToast
returned a callable function).
If the API surface is different (e.g.toast({ status: 'success', message: … })
), this will result in a runtimeTypeError
.Suggestion:
- toast.success('Logged out successfully') + toast?.success?.('Logged out successfully')or, better, adapt to the exact API exported by the
ToastProvider
.Please run a quick type-check / unit test to confirm these helpers exist.
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.
one blocker comment pls check; apart from that lgtm, approved
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.
Two issues:
(actually not a bug)
squid route failing handling:
bug: this userid (likely my address is in db relatedto user) 0xB92FCCCc63893856Da2a445d861f0942788D6479
https://peanut.me/claim?c=42161&v=v4.3&i=3761&t=ui#p=ee08BYdmYhcpYzvZ
contributes to TASK-11169
contributes to TASK-9923
Summary by CodeRabbit
New Features
Improvements
Refactor