Fix premature close of onramp #1163
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Vercel Unique URL: https://sendapp-bf8ys3s40-0xsend.vercel.app |
Playwright ReportSummary
Suitesaccount-rewards.onboarded.spec.tscan visit rewards page
account-sendtag-add.onboarded.spec.tscan visit add sendtags page
can add a pending tag
cannot add an invalid tag name
cannot add more than 5 tags
cannot confirm a tag without paying
account-sendtag-checkout.onboarded.spec.tscan confirm a tag
can refer a tag
can refer multiple tags in separate transactions
account-settings-backup.onboarded.spec.tscan backup account
can remove a signer
account.logged-in.spec.tscan visit account page
can update profile
activity.onboarded.spec.tscan visit activity page and see correct activity feed
can search on activity page
home.onboarded.spec.tscan visit token detail page
leaderboard.logged-in.spec.tscan visit leaderboard page
onboarding.logged-in.spec.tscan visit onboarding page
profile.anon.spec.tsanon user can visit public profile
anon user cannot visit private profile
profile.logged-in.spec.tslogged in user needs onboarding before visiting profile
profile.onboarded.spec.tscan visit other user profile and send by tag
can visit my own profile
can visit private profile
can view activities between another profile
send-token-upgrade.onboarded.spec.tscan upgrade their Send Token V0 to Send Token V1
send.onboarded.spec.tscan send USDC starting from profile page
can send USDC using tag starting from home page
can send USDC using sendid starting from home page
can send USDC using address starting from home page
can send ETH starting from profile page
can send ETH using tag starting from home page
can send ETH using sendid starting from home page
can send ETH using address starting from home page
can send SEND starting from profile page
can send SEND using tag starting from home page
can send SEND using sendid starting from home page
can send SEND using address starting from home page
sign-in.anon.spec.tsredirect on sign-in
redirect to send confirm page on sign-in
sign-up.anon.spec.tscan sign up
country code is selected based on geoip
skip otp for existing user trying to sign up using already used phone number
user can sign in with passkey from backing up page
user can sign in as back up using otp from backing up page
|
youngkidwarrior
requested changes
Mar 8, 2025
| jest.mock('app/features/deposit/components/PendingScreen', () => ({ | ||
| PendingScreen: () => <Text testID="pending-screen">PendingScreen-Mock</Text>, | ||
| })) | ||
| jest.mock('@my/ui', () => { |
There was a problem hiding this comment.
Ask @0xBigBoss if this is necessary. Seems like overkill
There was a problem hiding this comment.
This is probably mocking too much. We can use the test provider and we shouldn't have to mock the UI package. We can wrap the components in this provider and it should work without the excessive mocking.
| jest.clearAllMocks() | ||
|
|
||
| // Default mock implementations | ||
| ;(useSendAccount as unknown as jest.Mock).mockReturnValue(mockSendAccount) |
There was a problem hiding this comment.
Instead of using as here I'd use a @ts-expect-error comment
| * PendingScreen component that displays while waiting for a transaction to be confirmed. | ||
| * It polls for new USDC transactions and redirects to success page when a new transaction is detected. | ||
| * | ||
| * @param props - Component properties |
There was a problem hiding this comment.
These jsdoc comments not very useful
|
|
||
| // If we have new transactions, consider it a success | ||
| if (currentCount > initialActivityCount) { | ||
| // TODO: redirect to success page |
| useEffect(() => { | ||
| if (!activityData || initialActivityCount === null) return | ||
|
|
||
| const usdcToMe = activityData.pages[0]?.filter( |
There was a problem hiding this comment.
This logic doesn't make sense to me. Why are we comparing usdcToMe length to activity feed length
There was a problem hiding this comment.
we're trying to see if any new USDC deposits have been sent to the user.
| import { getOnrampBuyUrl } from '@coinbase/onchainkit/fund' | ||
| import { useMutation } from '@tanstack/react-query' | ||
| import { useRouter } from 'solito/router' | ||
|
|
||
| type OnrampStatus = 'idle' | 'pending' | 'success' | 'failed' | ||
| type OnrampStatus = 'idle' | 'pending_payment' | 'success' | 'failed' | 'payment_submitted' |
There was a problem hiding this comment.
Any reason we are changing 'pending' to 'pending_payment'
| @@ -127,17 +133,19 @@ export function useCoinbaseOnramp({ | |||
| const status: OnrampStatus = isSuccess | |||
There was a problem hiding this comment.
Can you use a switch(true) instead of a nested ternary
| @@ -22,25 +22,28 @@ export function useCoinbaseOnramp({ | |||
| partnerUserId, | |||
| defaultPaymentMethod = 'CARD', | |||
| }: OnrampConfig) { | |||
| const [popup, setPopup] = useState<Window | null>(null) | |||
| const popupRef = useRef<Window | null>(null) | |||
0xBigBoss
reviewed
Mar 9, 2025
| const [initialActivityCount, setInitialActivityCount] = useState<number | null>(null) | ||
| // Query to check for new USDC transactions | ||
| const { data: activityData } = useTokenActivityFeed({ | ||
| address: usdcAddress[baseMainnet.id] as PgBytea, |
There was a problem hiding this comment.
Need to use hexToBytea. Viem uses 0x prefix. Supabase uses \x prefix
There was a problem hiding this comment.
damn AI actually had it right and I went back and changed it bc it looked funny, noted
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.