Skip to content

[TASK-13586] fix: don't try to create account twice on setup#1160

Merged
jjramirezn merged 1 commit intopeanut-wallet-devfrom
fix/double-add-account
Sep 5, 2025
Merged

[TASK-13586] fix: don't try to create account twice on setup#1160
jjramirezn merged 1 commit intopeanut-wallet-devfrom
fix/double-add-account

Conversation

@jjramirezn
Copy link
Copy Markdown
Contributor

@jjramirezn jjramirezn commented Sep 1, 2025

Dont merge if approved, we are in code freeze

@notion-workspace
Copy link
Copy Markdown

@vercel
Copy link
Copy Markdown

vercel bot commented Sep 1, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
peanut-wallet Ready Ready Preview Comment Sep 1, 2025 7:28pm

@jjramirezn jjramirezn requested a review from Zishan-7 September 1, 2025 19:21
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Sep 1, 2025

Walkthrough

Introduces device-type awareness via a hook in Home, replaces userAgent iOS checks with DeviceType-based logic, and updates effect dependencies. Adds isFetchingUser guards in SetupPasskey and Home to prevent duplicate actions during user fetch, adjusts button states, and extends effect dependency arrays accordingly.

Changes

Cohort / File(s) Summary
Device-type/iOS PWA handling
src/app/(mobile-ui)/home/page.tsx
Uses useDeviceType() and DeviceType to detect iOS; replaces userAgent checks. Adds isFetchingUser guard and updates effect dependencies to [user, address, isFetchingUser]. Adds deviceType to iOS PWA modal effect dependencies.
Setup flow guards and UI state
src/components/Setup/Views/SetupPasskey.tsx
Imports AccountType. Introduces isFetchingUser from useAuth() to guard against duplicate account creation. Extends effect dependencies to include isFetchingUser. Button loading/disabled now consider isFetchingUser.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • Zishan-7
  • Hugo0
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/double-add-account

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/components/Setup/Views/SetupPasskey.tsx (1)

26-62: Fix: loading can get stuck and no redirect when account already exists

If the account already exists, the effect returns early so setLoading(false) never runs and no redirect occurs after a successful register. Clear loading and navigate when the wallet already exists.

 useEffect(() => {
-        // Dont try to double add the account
-        if (isFetchingUser || user?.accounts.some((a) => a.type === AccountType.PEANUT_WALLET)) return
+        // Don't try to double-add the account
+        if (isFetchingUser) return
+
+        // Already has Peanut Wallet: clear loading and redirect
+        if (user?.accounts.some((a) => a.type === AccountType.PEANUT_WALLET)) {
+            dispatch(setupActions.setLoading(false))
+            const localStorageRedirect = getFromLocalStorage('redirect')
+            if (localStorageRedirect) {
+                const matchedAction = POST_SIGNUP_ACTIONS.find((action) =>
+                    action.pathPattern.test(localStorageRedirect)
+                )
+                if (matchedAction) {
+                    router.push('/home')
+                } else {
+                    localStorage.removeItem('redirect')
+                    router.push(localStorageRedirect)
+                }
+            } else {
+                router.push('/home')
+            }
+            return
+        }
         if (address && user) {
             addAccount({
                 accountIdentifier: address,
                 accountType: WalletProviderType.PEANUT,
                 userId: user?.user.userId as string,
                 telegramHandle: telegramHandle.length > 0 ? telegramHandle : undefined,
             })
                 .then(() => {
                     const localStorageRedirect = getFromLocalStorage('redirect')
                     // redirect based on post signup action config
                     if (localStorageRedirect) {
                         const matchedAction = POST_SIGNUP_ACTIONS.find((action) =>
                             action.pathPattern.test(localStorageRedirect)
                         )
                         if (matchedAction) {
                             router.push('/home')
                         } else {
                             localStorage.removeItem('redirect')
                             router.push(localStorageRedirect)
                         }
                     } else {
                         router.push('/home')
                     }
                 })
                 .catch((e) => {
                     Sentry.captureException(e)
                     console.error('Error adding account', e)
                     setError('Error adding account')
                 })
                 .finally(() => {
                     dispatch(setupActions.setLoading(false))
                 })
         }
 }, [address, user, isFetchingUser])
src/app/(mobile-ui)/home/page.tsx (1)

98-103: Align accountType and add error handling

Use the same enum as SetupPasskey and handle rejection to avoid unhandled promise rejections.

-            addAccount({
-                accountIdentifier: address,
-                accountType: 'peanut-wallet',
-                userId: user.user.userId,
-            })
+            addAccount({
+                accountIdentifier: address,
+                accountType: WalletProviderType.PEANUT,
+                userId: user.user.userId,
+            }).catch((e) => {
+                Sentry.captureException(e)
+                console.error('Error adding account in Home()', e)
+            })

Additional diff (imports at top):

- import { AccountType } from '@/interfaces'
+ import { AccountType, WalletProviderType } from '@/interfaces'
+ import * as Sentry from '@sentry/nextjs'
🧹 Nitpick comments (2)
src/components/Setup/Views/SetupPasskey.tsx (1)

21-22: Avoid double useAuth() calls

Destructure all needed fields from a single useAuth() call to prevent redundant subscriptions.

-    const { user, isFetchingUser } = useAuth()
-    const { addAccount } = useAuth()
+    const { user, isFetchingUser, addAccount } = useAuth()
src/app/(mobile-ui)/home/page.tsx (1)

116-136: Future-proof useDeviceType for SSR safety (hook change, not in this file)

useGetDeviceType currently reads navigator.userAgent outside an effect. It’s fine here in a client page, but the hook itself can break if reused in a Server Component. Move UA detection inside useEffect and guard navigator.

--- a/src/hooks/useGetDeviceType.ts
+++ b/src/hooks/useGetDeviceType.ts
@@
 export const useDeviceType = () => {
     const [deviceType, setDeviceType] = useState<DeviceType | null>(null)
-
-    const isIos = /iPad|iPhone|iPod/.test(navigator.userAgent)
-    const isAndroid = /android/i.test(navigator.userAgent)
-
-    // check if the user is on ios or android
-    useEffect(() => {
-        if (isIos) {
-            setDeviceType(DeviceType.IOS)
-        } else if (isAndroid) {
-            setDeviceType(DeviceType.ANDROID)
-        } else {
-            setDeviceType(DeviceType.WEB)
-        }
-    }, [])
+    useEffect(() => {
+        if (typeof navigator === 'undefined') {
+            setDeviceType(DeviceType.WEB)
+            return
+        }
+        const ua = navigator.userAgent || ''
+        const isIos = /iPad|iPhone|iPod/.test(ua)
+        const isAndroid = /android/i.test(ua)
+        if (isIos) setDeviceType(DeviceType.IOS)
+        else if (isAndroid) setDeviceType(DeviceType.ANDROID)
+        else setDeviceType(DeviceType.WEB)
+    }, [])
 
     return { deviceType }
 }
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 24eb5b3 and 0a8e069.

📒 Files selected for processing (2)
  • src/app/(mobile-ui)/home/page.tsx (6 hunks)
  • src/components/Setup/Views/SetupPasskey.tsx (3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-20T09:08:19.266Z
Learnt from: kushagrasarathe
PR: peanutprotocol/peanut-ui#1112
File: src/components/Claim/Link/views/BankFlowManager.view.tsx:336-343
Timestamp: 2025-08-20T09:08:19.266Z
Learning: In the KYC flow implementation, `setJustCompletedKyc` must be called after `await fetchUser()` in the `handleKycSuccess` callback. Setting `justCompletedKyc` before fetching the user would cause a re-fetching loop because `handleKycSuccess` is set in a useEffect inside the KYC hook, which would cause the UI flow to get stuck in one view.

Applied to files:

  • src/app/(mobile-ui)/home/page.tsx
📚 Learning: 2024-12-02T17:19:18.532Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#551
File: src/components/Request/Create/Views/Initial.view.tsx:151-156
Timestamp: 2024-12-02T17:19:18.532Z
Learning: In the `InitialView` component at `src/components/Request/Create/Views/Initial.view.tsx`, when setting the default chain and token in the `useEffect` triggered by `isPeanutWallet`, it's acceptable to omit the setters from the dependency array and not include additional error handling for invalid defaults.

Applied to files:

  • src/components/Setup/Views/SetupPasskey.tsx
🧬 Code graph analysis (2)
src/app/(mobile-ui)/home/page.tsx (2)
src/hooks/useGetDeviceType.ts (1)
  • useDeviceType (14-32)
src/utils/general.utils.ts (1)
  • isIOS (1270-1275)
src/components/Setup/Views/SetupPasskey.tsx (2)
src/context/authContext.tsx (1)
  • useAuth (182-188)
src/components/0_Bruddle/Button.tsx (1)
  • Button (76-267)
⏰ 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 (5)
src/components/Setup/Views/SetupPasskey.tsx (2)

69-71: LGTM: button state includes isFetchingUser

Disabling and showing loading while the user is fetching prevents duplicate actions.


30-35: Ignore accountType consistency concern
WalletProviderType.PEANUT resolves to the string 'peanut-wallet', matching the literal used in Home(); no changes required.

Likely an incorrect or invalid review comment.

src/app/(mobile-ui)/home/page.tsx (3)

93-105: LGTM: guarded wallet creation

Early return on isFetchingUser avoids premature addAccount and double-creation.


116-136: LGTM: deviceType-based iOS check and deps

Switching to deviceType and adding it to the dependency array is cleaner and avoids UA checks here.


98-103: Guard in-flight addAccount calls
If addAccount isn’t guaranteed idempotent server-side, track and block in-flight requests (e.g. disable while pending) to prevent duplicate API calls on rapid re-renders.

Copy link
Copy Markdown
Contributor

@Zishan-7 Zishan-7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jjramirezn jjramirezn merged commit 5d0d5fd into peanut-wallet-dev Sep 5, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants