refactor(payments): unify SplitDetailPage with shared checkout logic#545
Conversation
|
@Nanfe01 is attempting to deploy a commit to the olufunbiik's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughA new React hook Changes
Sequence DiagramsequenceDiagram
participant Caller as Component
participant Hook as usePaymentCheckout
participant Wallet as Wallet Validator
participant Network as Network Validator
participant API as Payment API
Caller->>Hook: submitPayment(params)
Hook->>Hook: Check if loading or duplicate
alt Already Processing or Duplicate
Hook->>Caller: Return early
else Proceed
Hook->>Wallet: isWalletConnected()
Wallet-->>Hook: Connected status
Hook->>Network: isCorrectNetwork()
Network-->>Hook: Network status
alt Validation Failed
Hook->>Hook: Set error state
Hook->>Caller: Return
else Validation Passed
Hook->>Hook: Set loading=true
Hook->>API: Execute payment request
API-->>Hook: Success/Response
Hook->>Hook: Log success, update lastSubmittedId
Hook->>Caller: Return with data
end
Hook->>Hook: Clear loading state (finally)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/usePaymentCheckout.ts`:
- Around line 3-6: The hook in frontend/src/components/usePaymentCheckout.ts
currently hardcodes network validity, ignores the CheckoutParams.amount, and
returns a simulated success; replace the stubbed logic by delegating to the
shared payment flow: call the shared hook/function from
frontend/src/hooks/usePaymentCheckout.ts (or the wallet submit/transaction
helpers in frontend/src/utils/stellar/wallet.ts) to validate network, build and
submit the on‑chain payment using the provided amount and splitId, and propagate
real success/error responses instead of the simulated 200 response; ensure you
remove the hardcoded network check and the fake success branch and forward any
errors from the wallet helper back to callers.
- Around line 26-27: Replace the asynchronous state-based duplicate-submit guard
in usePaymentCheckout with a synchronous ref-based in-flight guard: introduce a
useRef (e.g., inFlightRef) and check/flip it immediately at the top of the
submit handler (instead of relying on loading or lastSubmittedId) to prevent
race conditions; continue to update lastSubmittedId (and loading state) for UI,
but ensure you set lastSubmittedId only after a successful submit, and always
clear inFlightRef and reset loading inside the catch/finally path so failed
attempts can be retried (adjust references to splitId, setLastSubmittedId,
setLoading, and existing try/catch blocks accordingly).
- Around line 13-16: The hook currently reads window?.ethereum.selectedAddress
directly in isWalletConnected and computes snapshot values at initialization,
causing SSR errors and stale state; make isWalletConnected and isCorrectNetwork
SSR-safe by guarding with typeof window !== 'undefined' before accessing
window.ethereum and add a proper Window/Ethereum type augmentation for the
injected provider; instead of returning eager boolean snapshots (the current
return object calling isWalletConnected() and isCorrectNetwork()), either return
the functions themselves or implement useState + useEffect to subscribe to
provider events (e.g., accountsChanged, chainChanged) so the hook updates
reactively (update function names isWalletConnected, isCorrectNetwork and any
return shape accordingly).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 70ebb16d-b325-4e68-b91c-0e5581bcecba
📒 Files selected for processing (1)
frontend/src/components/usePaymentCheckout.ts
| type CheckoutParams = { | ||
| amount: number; | ||
| splitId: string; | ||
| }; |
There was a problem hiding this comment.
This hook currently “succeeds” without submitting a real payment transaction.
Line 20 hardcodes network validity, Lines 46-49 simulate success, and amount is unused (Line 24). This diverges from the shared payment contract and can mark payments as successful without on-chain/API submission. Please wire this to the existing checkout/wallet flow (frontend/src/hooks/usePaymentCheckout.ts:1-77 and frontend/src/utils/stellar/wallet.ts:49-119) before release.
💡 Minimal alignment option (delegate to shared hook)
-import { useState } from "react";
-
-type CheckoutParams = {
- amount: number;
- splitId: string;
-};
-
-export const usePaymentCheckout = () => {
- ...
-};
+export { usePaymentCheckout } from "../hooks/usePaymentCheckout";Also applies to: 18-21, 45-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/usePaymentCheckout.ts` around lines 3 - 6, The hook
in frontend/src/components/usePaymentCheckout.ts currently hardcodes network
validity, ignores the CheckoutParams.amount, and returns a simulated success;
replace the stubbed logic by delegating to the shared payment flow: call the
shared hook/function from frontend/src/hooks/usePaymentCheckout.ts (or the
wallet submit/transaction helpers in frontend/src/utils/stellar/wallet.ts) to
validate network, build and submit the on‑chain payment using the provided
amount and splitId, and propagate real success/error responses instead of the
simulated 200 response; ensure you remove the hardcoded network check and the
fake success branch and forward any errors from the wallet helper back to
callers.
| const isWalletConnected = () => { | ||
| // replace with real wallet hook | ||
| return Boolean(window?.ethereum?.selectedAddress); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Check whether Window.ethereum is typed anywhere
rg -n --type=ts --type=tsx 'interface\s+Window|declare\s+global|ethereum' -g '!**/node_modules/**'
# 2) Locate direct window.ethereum access patterns
rg -n --type=ts --type=tsx '\bwindow(\?|\.)' -g '!**/node_modules/**'
# 3) Confirm which usePaymentCheckout implementation is imported by consumers
rg -n --type=ts --type=tsx 'from\s+["'\''][^"'\'']*usePaymentCheckout["'\'']'Repository: OlufunbiIK/StellarSplit
Length of output: 95
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# List files in frontend/src/components to understand structure
ls -la frontend/src/components/ 2>/dev/null | head -20
# Check if the file exists
find . -name "usePaymentCheckout.ts" -type fRepository: OlufunbiIK/StellarSplit
Length of output: 1352
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for Window interface/ethereum typing (adjusted for ts files only)
rg -n 'interface\s+Window|declare\s+global|ethereum' --type ts -g '!**/node_modules/**' 2>/dev/null || echo "No matches found"
# Search for window access patterns
rg -n 'window\s*[\?\.]' --type ts -g '!**/node_modules/**' 2>/dev/null | head -30 || echo "No matches found"
# Search for usePaymentCheckout imports
rg -n 'usePaymentCheckout' --type ts -g '!**/node_modules/**' 2>/dev/null || echo "No matches found"Repository: OlufunbiIK/StellarSplit
Length of output: 3648
🏁 Script executed:
cat frontend/src/components/usePaymentCheckout.tsRepository: OlufunbiIK/StellarSplit
Length of output: 1540
window?.ethereum usage creates SSR risk and requires explicit typing.
Line 15 can fail in server-side runtime environments, and window.ethereum is not a standard Web API—it's injected by wallet providers and needs explicit Window augmentation for TypeScript.
Also, lines 61-62 return eager snapshots of wallet state (calling isWalletConnected() and isCorrectNetwork() immediately in the return object). These values are computed once at hook initialization and won't update if the wallet connection or network changes. Return the functions themselves or use state/effects to track reactive wallet state:
SSR-safe pattern
const isWalletConnected = () => {
+ if (typeof window === "undefined") return false;
- return Boolean(window?.ethereum?.selectedAddress);
+ const eth = (window as Window & { ethereum?: { selectedAddress?: string } }).ethereum;
+ return Boolean(eth?.selectedAddress);
};Return functions in hook output:
return {
submitPayment,
loading,
error,
- isWalletConnected: isWalletConnected(),
- isCorrectNetwork: isCorrectNetwork(),
+ isWalletConnected,
+ isCorrectNetwork,
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/usePaymentCheckout.ts` around lines 13 - 16, The hook
currently reads window?.ethereum.selectedAddress directly in isWalletConnected
and computes snapshot values at initialization, causing SSR errors and stale
state; make isWalletConnected and isCorrectNetwork SSR-safe by guarding with
typeof window !== 'undefined' before accessing window.ethereum and add a proper
Window/Ethereum type augmentation for the injected provider; instead of
returning eager boolean snapshots (the current return object calling
isWalletConnected() and isCorrectNetwork()), either return the functions
themselves or implement useState + useEffect to subscribe to provider events
(e.g., accountsChanged, chainChanged) so the hook updates reactively (update
function names isWalletConnected, isCorrectNetwork and any return shape
accordingly).
| // 🚫 prevent duplicate submits | ||
| if (loading || lastSubmittedId === splitId) return; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n frontend/src/components/usePaymentCheckout.tsRepository: OlufunbiIK/StellarSplit
Length of output: 1988
Race condition in duplicate-submit protection allows double-submits.
The guard at line 27 checks state values, but React state updates are asynchronous. Two rapid calls can both pass the check before the first call's state updates are committed, causing both to proceed to the try block and submit duplicate payments.
Additionally, lastSubmittedId is set at line 43 but never reset on error (line 50-51), blocking retries of failed payments.
Solution: Use useRef for an in-flight flag to make the guard atomic with immediate synchronous state:
Fix with useRef
-import { useState } from "react";
+import { useRef, useState } from "react";
...
+ const inFlightRef = useRef(false);
...
- if (loading || lastSubmittedId === splitId) return;
+ if (inFlightRef.current) return;
...
try {
+ inFlightRef.current = true;
setLoading(true);
setLastSubmittedId(splitId);
...
} catch (e: any) {
setError(e.message || "Payment failed");
+ setLastSubmittedId(null);
} finally {
+ inFlightRef.current = false;
setLoading(false);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/usePaymentCheckout.ts` around lines 26 - 27, Replace
the asynchronous state-based duplicate-submit guard in usePaymentCheckout with a
synchronous ref-based in-flight guard: introduce a useRef (e.g., inFlightRef)
and check/flip it immediately at the top of the submit handler (instead of
relying on loading or lastSubmittedId) to prevent race conditions; continue to
update lastSubmittedId (and loading state) for UI, but ensure you set
lastSubmittedId only after a successful submit, and always clear inFlightRef and
reset loading inside the catch/finally path so failed attempts can be retried
(adjust references to splitId, setLastSubmittedId, setLoading, and existing
try/catch blocks accordingly).
2 participants
refactor(payments): unify SplitDetailPage with shared checkout logic
closes #464
Summary by CodeRabbit