-
Notifications
You must be signed in to change notification settings - Fork 13
Prod Release #579
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 #579
Conversation
[TASK-7005] fix: migrate web3Modal to reown-appkit
…into feat/promo-checker
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Warning Rate limit exceeded@Hugo0 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
WalkthroughThis pull request focuses on migrating the wallet connection mechanism from Changes
Sequence DiagramsequenceDiagram
participant User
participant App
participant AppKit
participant WalletConnector
User->>App: Initiate Wallet Connection
App->>AppKit: Request Wallet Modal
AppKit->>WalletConnector: Open Wallet Selection
WalletConnector-->>User: Display Wallet Options
User->>WalletConnector: Select Wallet
WalletConnector->>App: Wallet Connected
App->>User: Update UI with Connected Wallet
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (5)
src/components/Global/Header/index.tsx (1)
Line range hint
278-289
: Consider consolidating wallet connection logicThe wallet connection logic is duplicated between MenuLinks and SocialLinks components.
Consider extracting the wallet connection logic into a custom hook:
+// src/hooks/useWalletConnection.ts +import { useAppKit } from '@reown/appkit/react' +import { useAccount } from 'wagmi' + +export const useWalletConnection = () => { + const { open: walletModal } = useAppKit() + const { address, isConnected } = useAccount() + + return { walletModal, address, isConnected } +}src/components/Create/Link/Input.view.tsx (2)
Line range hint
74-77
: Consider memoizing the handleConnectWallet functionSince this function is used in multiple event handlers, it should be memoized to prevent unnecessary re-renders.
-const handleConnectWallet = async () => { - open() -} +const handleConnectWallet = useCallback(async () => { + open() +}, [open])
Line range hint
78-267
: Consider breaking down the handleOnNext functionThe handleOnNext function is quite large and handles multiple responsibilities. This could make it harder to maintain and test.
Consider breaking it down into smaller, focused functions:
- validateAndPrepare
- handleGaslessDeposit
- handleRegularDeposit
- handleDirectSend
This would improve readability and make the code easier to test and maintain.
src/components/Global/GeneralRecipientInput/index.tsx (2)
Line range hint
34-73
: Consider refactoring validation logic for better maintainabilityThe
checkAddress
function handles multiple recipient types with complex validation logic. Consider extracting type-specific validation into separate functions for better maintainability.Example refactor:
const validateEthereumAddress = (address: string): boolean => { return isAddress(address) } const validateENSName = async (name: string): Promise<{isValid: boolean, resolvedAddress?: string}> => { const address = await utils.resolveFromEnsName(name.toLowerCase()) return { isValid: !!address, resolvedAddress: address } } const validateBankAccount = async (account: string): Promise<{isValid: boolean, type: RecipientType}> => { const sanitizedInput = sanitizeBankAccount(account) if (isIBAN(sanitizedInput)) { const isValid = await utils.validateBankAccount(sanitizedInput) return { isValid, type: 'iban' } } if (/^[0-9]{1,17}$/.test(sanitizedInput)) { return { isValid: true, type: 'us' } } return { isValid: false, type: 'address' } }
Line range hint
1-140
: Document the validation changes in component propsGiven these are shared components with significant validation changes, consider updating the component's props documentation to clearly specify the new validation rules.
Example documentation:
/** * @property recipient - The recipient information * @property recipient.address - Must be a valid Ethereum address in checksum format * @property recipient.name - Optional ENS name that resolves to a valid address */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
package.json
(4 hunks)src/components/Cashout/Components/Initial.view.tsx
(2 hunks)src/components/Claim/Link/Initial.view.tsx
(2 hunks)src/components/Create/Link/Input.view.tsx
(2 hunks)src/components/Global/AddressInput/index.tsx
(1 hunks)src/components/Global/GeneralRecipientInput/index.tsx
(1 hunks)src/components/Global/Header/index.tsx
(5 hunks)src/components/Global/TokenSelector/TokenSelector.tsx
(2 hunks)src/components/Profile/Components/SkeletonPage.tsx
(2 hunks)src/components/Refund/index.tsx
(1 hunks)src/components/Request/Pay/Views/Initial.view.tsx
(2 hunks)src/config/wagmi.config.tsx
(1 hunks)
🧰 Additional context used
📓 Learnings (1)
src/components/Request/Pay/Views/Initial.view.tsx (1)
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#422
File: src/components/Request/Pay/Views/Initial.view.tsx:76-78
Timestamp: 2024-11-12T09:39:20.720Z
Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(estimatedGasCost, 3)` return strings, ensuring consistent return types for `calculatedFee`.
🔇 Additional comments (28)
src/config/wagmi.config.tsx (8)
4-5
: Imports look correct and align with the new wallet approach.
No issues found. Make sure the packages are installed and the imports are valid in other usages as well.
13-13
: Validate environment variables.
Ensure that sensitive or critical environment variables (like project IDs) are handled properly across environments.
20-20
: Confirm domain alignment.
The domain in the metadata URL should match the actual production domain to avoid potential mismatch or CORS issues.
24-26
: Transport setup looks sound.
Creating transport mappings for each chain is a clean approach that simplifies chain-based RPC management.
28-43
: Safe domain list.
The allowed domains array for Safe connectors is properly specified. Double-check if you need wildcard coverage or additional domain patterns.
45-50
: WagmiAdapter construction is clear.
The SSR set to true is helpful for Next.js. Confirm that SSR usage does not conflict with any client-side only dependencies.
54-65
: AppKit features.
Enabling analytics, onramp, and other features is straightforward. Looks good for an initial setup.
74-74
: Check integration tests for the final WagmiProvider config.
Make sure the new config is tested for all functionalities, including wallet switching and transaction execution.
src/components/Claim/Link/Initial.view.tsx (2)
7-7
: useAppKit import.
The import from '@reown/appkit/react' is correct and aligns with the new approach.
87-87
: Destructure 'open' from useAppKit.
Using the 'open' method inside handleConnectWallet is a valid replacement for the old Web3Modal approach.
package.json (7)
31-32
: New dependencies for Reown AppKit.
The addition of '@reown/appkit' and '@reown/appkit-adapter-wagmi' is consistent with the wallet migration.
36-36
: React Query version update.
Ensure the codebase is compatible with React Query 5.8.4 and that no breaking changes are introduced.
39-39
: Downgraded @wagmi/core version.
Downgrading from ^2.14.6 to 2.14.3 might be intentional for compatibility; verify in test.
70-71
: Testing libraries (@testing-library/jest-dom, @types/jest).
Keeps the testing environment up-to-date. Confirm usage with new wallet workflows.
80-81
: Added jest and jest-environment-jsdom.
Essential for React component testing. Ensure coverage includes new wallet logic.
88-89
: ts-jest and TypeScript version update.
ts-jest ^29.1.2 is usually compatible with TypeScript 5.6.3. Double-check no type conflicts across the codebase.
104-108
: Addition of resolutions field.
Locking versions for @wagmi/core, viem, and wagmi can avoid dependency conflicts. Confirm no features are lost from pinned versions.
src/components/Refund/index.tsx (2)
13-13
: useAppKit import.
Migrating from Web3Modal to useAppKit is consistent with the rest of the PR changes.
19-19
: Extracting 'open' from useAppKit.
No issues found. This ensures a single approach to wallet connection across the app.
src/components/Global/Header/index.tsx (2)
9-9
: LGTM: Import statement updated correctly
The import statement has been properly updated to use the new AppKit library.
Line range hint 225-262
: Verify wallet connection behavior in MenuLinks
The wallet connection logic has been correctly migrated to use AppKit. However, let's verify that there are no remaining references to the old Web3Modal in the codebase.
✅ Verification successful
No Web3Modal references found in the codebase
The search results confirm that there are no remaining references to Web3Modal in any TypeScript or TypeScript React files. The wallet connection logic has been successfully migrated to use AppKit without any legacy dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining Web3Modal references
rg -i "web3modal" --type ts --type tsx
Length of output: 66
Script:
#!/bin/bash
# Search for any remaining Web3Modal references with proper file extensions
rg -i "web3modal" -g "*.ts" -g "*.tsx"
# Also search for common Web3Modal import patterns
rg -i "from ['\"]@web3modal" -g "*.ts" -g "*.tsx"
rg -i "import.*web3modal" -g "*.ts" -g "*.tsx"
Length of output: 141
src/components/Profile/Components/SkeletonPage.tsx (1)
4-4
: LGTM: AppKit integration is correct
The migration to AppKit has been implemented correctly, maintaining the same functionality for wallet connection.
Also applies to: 31-31
src/components/Create/Link/Input.view.tsx (1)
6-6
: LGTM: AppKit integration is correct
The migration to AppKit has been implemented correctly. The wallet connection logic is properly integrated with the component's functionality.
Also applies to: 72-72
src/components/Global/TokenSelector/TokenSelector.tsx (1)
16-16
: LGTM: Wallet connection migration looks good.
The changes correctly migrate the wallet connection from @web3modal/wagmi
to @reown/appkit
while preserving the component's functionality.
Also applies to: 189-189
src/components/Cashout/Components/Initial.view.tsx (1)
7-7
: LGTM: Wallet connection migration looks good.
The changes correctly migrate the wallet connection from @web3modal/wagmi
to @reown/appkit
while preserving the component's functionality.
Also applies to: 74-74
src/components/Request/Pay/Views/Initial.view.tsx (1)
3-3
: LGTM: Wallet connection migration looks good.
The changes correctly migrate the wallet connection from @web3modal/wagmi
to @reown/appkit
while preserving the component's functionality. The retrieved learning confirms that both txFee
and utils.formatTokenAmount(estimatedGasCost, 3)
return strings, ensuring consistent return types for calculatedFee
.
Also applies to: 73-73
src/components/Global/AddressInput/index.tsx (1)
21-21
:
Verify impact of stricter address validation
The removal of { strict: false }
from isAddress
makes address validation stricter, which could reject addresses that were previously accepted. This is a potentially breaking change that requires careful consideration.
Let's verify the impact:
Consider:
- Adding tests to verify the new validation behavior
- Documenting this change in release notes
- Implementing a migration strategy if needed
src/components/Global/GeneralRecipientInput/index.tsx (1)
66-68
:
Ensure consistent address validation across components
The address validation change here matches the change in AddressInput
, but this component handles multiple recipient types. Consider:
- The error message might need to be more specific about validation requirements
- The validation behavior should be consistent with other components
Let's verify the consistency:
✅ Verification successful
Let me gather more information about the validation patterns across these components.
Let me try a different approach to analyze the validation patterns.
Address validation is consistently implemented across components
The validation behavior is consistent between AddressInput
and GeneralRecipientInput
:
- Both components use
isAddress
from 'viem' without additional parameters GeneralRecipientInput
has appropriate error handling for different recipient types (ENS, IBAN, US account, Ethereum address)- The error message "Invalid Ethereum address" is specific and appropriate as it's used only when validating Ethereum addresses, while other recipient types have their own specific messages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all components that use address validation
ast-grep --pattern 'isAddress($$$)'
# Check for any inconsistencies in error messages
rg "Invalid.*address|Invalid.*ethereum"
Length of output: 912
Script:
#!/bin/bash
# Check the validation logic in AddressInput and GeneralRecipientInput
ast-grep --pattern 'function $$$($$$) {
$$$
isAddress($$$)
$$$
}'
# Look for other validation methods that might be related
rg -A 3 "function.*validate" src/components/Global/
Length of output: 127
Script:
#!/bin/bash
# Get more context around isAddress usage in both components
rg -B 3 -A 3 "isAddress" src/components/Global/GeneralRecipientInput/index.tsx
rg -B 3 -A 3 "isAddress" src/components/Global/AddressInput/index.tsx
# Look for any other validation-related code in these files
rg -B 2 -A 2 "isValid|validate|error" src/components/Global/GeneralRecipientInput/index.tsx
rg -B 2 -A 2 "isValid|validate|error" src/components/Global/AddressInput/index.tsx
Length of output: 4604
[TASK-7535] feat: add promo-code checker
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: 5
🔭 Outside diff range comments (1)
src/components/Global/Icon/index.tsx (1)
Line range hint
208-211
: Improve type safety for thename
propThe
name
prop is typed asany
, which bypasses TypeScript's type checking. Consider using a union type of available icon names for better type safety.type IconProps = { className?: string - name: any + name: keyof typeof icons fill?: string onClick?: () => void }
🧹 Nitpick comments (7)
src/components/Global/Icon/index.tsx (3)
Line range hint
213-225
: Consider memoizing the Icon componentSince icons are often used in lists or frequently re-rendered components, consider memoizing the component to prevent unnecessary re-renders.
+ import { memo } from 'react' const Icon = ({ className, name, fill, onClick }: IconProps) => ( <svg className={twMerge(`inline-block h-4 w-4 ${className}`)} width={16} height={16} viewBox={customViewbox.find((box) => box.name === name)?.viewBox ?? '0 0 16 16'} style={customStyle.find((style) => style.name === name)?.style ?? undefined} onClick={onClick} > <path fill={fill} d={icons[name]}></path> </svg> ) - export default Icon + export default memo(Icon)
Line range hint
7-39
: Optimize lookups using objects instead of arraysThe current implementation uses
find
operations on arrays for viewBox and style lookups, which have O(n) complexity. Consider using objects for O(1) lookup performance.- const customViewbox = [ - { name: 'gas', viewBox: '0 0 24 24 ' }, - // ... other entries - ] - const customStyle = [ - { - name: 'peanut', - style: { - // ... style object - }, - }, - ] + const customViewbox: Record<string, string> = { + gas: '0 0 24 24', + // ... other entries + } + const customStyle: Record<string, React.CSSProperties> = { + peanut: { + // ... style object + }, + } // Then in the component: - viewBox={customViewbox.find((box) => box.name === name)?.viewBox ?? '0 0 16 16'} - style={customStyle.find((style) => style.name === name)?.style ?? undefined} + viewBox={customViewbox[name] ?? '0 0 16 16'} + style={customStyle[name] ?? undefined}
Line range hint
208-211
: Add validation for the fill propTo prevent potential SVG injection attacks and ensure valid colors, consider adding validation for the
fill
prop.+ const isValidColor = (color: string) => { + const s = new Option().style; + s.color = color; + return s.color !== ''; + } type IconProps = { className?: string name: keyof typeof icons - fill?: string + fill?: string & { _brand: 'ValidColor' } onClick?: () => void } const Icon = ({ className, name, fill, onClick }: IconProps) => { + if (fill && !isValidColor(fill)) { + console.warn(`Invalid color value: ${fill}`); + return null; + } // ... rest of the component }src/app/api/peanut/submit-cashout-link/route.ts (1)
29-37
: Consider adding type safety to API request bodyThe request body is using object shorthand notation which is good, but consider adding type safety using zod or similar validation library for the API route.
Example type definition:
import { z } from 'zod' const CashoutRequestSchema = z.object({ bridgeCustomerId: z.string(), liquidationAddressId: z.string(), cashoutTransactionHash: z.string(), externalAccountId: z.string(), chainId: z.number(), tokenName: z.string(), pubKey: z.string(), promoCode: z.string().optional(), trackParam: z.string().optional() })src/components/Offramp/PromoCodeChecker.tsx (1)
91-99
: Add accessibility attributes to the input fieldEnhance the input field with proper accessibility attributes.
<input type="text" value={promoCheckerState.code} onChange={(e) => setPromoCheckerState((prev) => ({ ...prev, code: e.target.value }))} placeholder="Enter promo code" + aria-label="Promo code input" + aria-invalid={!!promoCheckerState.error} + aria-describedby={promoCheckerState.error ? "promo-error" : undefined} className={`custom-input h-8 px-3 text-xs transition-all duration-300 ${ promoCheckerState.error ? 'border border-red' : '' }`} />src/components/Offramp/Success.view.tsx (1)
92-96
: Extract fee description logicConsider extracting the fee description logic to improve readability and maintainability.
+const getFeeDescription = (appliedPromoCode: string | null | undefined, accountType: string | undefined) => { + if (appliedPromoCode) return `Fees waived with promo code ${appliedPromoCode}` + + const primaryFee = accountType === 'iban' ? 'SEPA transactions a fee of $1' : 'ACH transactions a fee of $0.50' + const secondaryFee = accountType === 'iban' ? 'ACH transactions a fee of $0.50' : 'SEPA transactions a fee of $1' + return `For ${primaryFee} is charged. For ${secondaryFee} is charged. This will be deducted of the amount you will receive.` +} -text={ - appliedPromoCode - ? `Fees waived with promo code ${appliedPromoCode}` - : accountType === 'iban' - ? 'For SEPA transactions a fee of $1 is charged. For ACH transactions a fee of $0.50 is charged. This will be deducted of the amount you will receive.' - : 'For ACH transactions a fee of $0.50 is charged. For SEPA transactions a fee of $1 is charged. This will be deducted of the amount you will receive.' -} +text={getFeeDescription(appliedPromoCode, accountType)}src/utils/cashout.utils.ts (1)
526-527
: Add JSDoc documentation for the new parameters.The implementation of the promo code and tracking parameters looks good. Consider adding JSDoc documentation to describe the purpose and format of these new parameters.
Add documentation like this:
+/** + * Submits a cashout link to the API. + * @param data.promoCode - Optional promotional code to apply discounts + * @param data.trackParam - Optional parameter for tracking the promo code usage + */Also applies to: 547-548
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
src/app/api/peanut/submit-cashout-link/route.ts
(2 hunks)src/components/Cashout/Cashout.consts.ts
(3 hunks)src/components/Claim/Link/FlowManager.tsx
(2 hunks)src/components/Global/Icon/index.tsx
(1 hunks)src/components/Offramp/Confirm.view.tsx
(6 hunks)src/components/Offramp/Offramp.consts.ts
(1 hunks)src/components/Offramp/PromoCodeChecker.tsx
(1 hunks)src/components/Offramp/Success.view.tsx
(2 hunks)src/utils/cashout.utils.ts
(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Offramp/PromoCodeChecker.tsx
[error] 40-47: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
🔇 Additional comments (9)
src/components/Global/Icon/index.tsx (1)
205-206
: New icons added successfully!
The ticket
and chevron-up
icons have been properly added with valid SVG path data, following the established pattern.
src/components/Claim/Link/FlowManager.tsx (2)
22-22
: LGTM: Type definition properly extends IClaimScreenProps
The optional appliedPromoCode
field is correctly typed as string | null
and properly extends the existing props interface.
36-37
: Verify props spreading maintains backward compatibility
The props spreading looks correct, but let's verify that existing components can handle the new appliedPromoCode
prop.
✅ Verification successful
Props spreading is safe and properly handled
The appliedPromoCode
prop is already well-integrated across the codebase:
- It's properly typed in multiple locations including
Offramp.consts.ts
andCashout.consts.ts
- The prop is consistently typed as
string | null
with optional modifier - It's actively used in
Success.view.tsx
andConfirm.view.tsx
with proper null checks - The implementation in
FlowManager.tsx
follows the existing pattern
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components that receive these props to ensure they handle the new field
ast-grep --pattern 'interface $interface {
$$$
appliedPromoCode?: string
$$$
}'
# Check for potential undefined handling
rg -l "appliedPromoCode" | xargs rg "(\?\.|\!\.)appliedPromoCode"
Length of output: 253
Script:
#!/bin/bash
# Search for components that receive these props to ensure they handle the new field
ast-grep --pattern 'interface $interface {
$$$
appliedPromoCode?: string
$$$
}'
# Search for usage of appliedPromoCode
rg "appliedPromoCode"
# Search for prop type definitions that might include appliedPromoCode
ast-grep --pattern 'type $type = {
$$$
appliedPromoCode?: string
$$$
}'
Length of output: 1852
src/app/api/peanut/submit-cashout-link/route.ts (1)
17-18
: Verify input validation for new parameters
New parameters promoCode
and trackParam
have been added to the request body. Ensure these are properly validated before being sent to the API.
src/components/Cashout/Cashout.consts.ts (2)
Line range hint 23-27
: LGTM: Improved type safety in CASHOUT_SCREEN_MAP
The change from React.FC<any>
to React.FC<ICashoutScreenProps>
improves type safety by ensuring components receive the correct props.
68-69
: Verify implementation of required offrampType prop
The offrampType
is added as a required prop. Let's verify all components implement this new required prop correctly.
✅ Verification successful
No issues found with offrampType implementation
The verification shows that:
- The
offrampType
prop is correctly defined as required inICashoutScreenProps
- The component using this interface (
InitialCashoutView
) is part of theCASHOUT_SCREEN_MAP
where all components are typed withICashoutScreenProps
- The implementation is type-safe as TypeScript would enforce the required prop at compile time
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components that should implement the new required prop
ast-grep --pattern 'const $component = ($props: ICashoutScreenProps) => {
$$$
}'
# Check if offrampType is being passed to these components
rg -A 3 "offrampType={"
Length of output: 200
Script:
#!/bin/bash
# Search for components using ICashoutScreenProps type
ast-grep --pattern '$_: ICashoutScreenProps'
# Search for usage of offrampType prop
rg "offrampType="
# Search for files importing ICashoutScreenProps
rg "ICashoutScreenProps"
Length of output: 404
Script:
#!/bin/bash
# Let's check the Initial.view.tsx implementation and its usage
cat src/components/Cashout/Components/Initial.view.tsx
# Check the CASHOUT_SCREEN_MAP implementation to see how components are used
cat src/components/Cashout/Cashout.consts.ts
# Search for any usage of Initial.view component
rg "Initial\.view"
Length of output: 22928
src/components/Offramp/Offramp.consts.ts (1)
79-79
: LGTM: Type definition is correct
The optional appliedPromoCode
property is properly typed as string | null
, which aligns well with its usage in the component.
src/components/Offramp/Confirm.view.tsx (2)
73-76
: LGTM: Clean state management implementation.
The promo code state management follows React best practices with proper typing and a focused handler function.
Also applies to: 100-103
863-863
: LGTM: Clean integration of PromoCodeChecker component.
The PromoCodeChecker component is well-integrated with proper prop passing and sensible placement in the UI.
export const VALID_PROMO_CODES = [ | ||
'XMAS', | ||
'GONUTS', | ||
'PEANUTFREN', | ||
'CLUBPEANUT', | ||
'DISCORD', | ||
'W3HUB', | ||
'MINDSPACE', | ||
'FULLNODE', | ||
'HUCKLETREE', | ||
'THURSDAO', | ||
'STATIONF', | ||
'HEDEN', | ||
'THEBLOCK', | ||
'BASEDINLISBON', | ||
'THEFINTECHHOUSE', | ||
'KUBECOWORKING', | ||
'CVLABS', | ||
'ATICOLAB', | ||
'UNYTED', | ||
'WEB3FAMILY', | ||
'ONECOWORK', | ||
'LAVACACOWORKING', | ||
'CRYPTODAYS', | ||
'SECONDCULTURE', | ||
'DECENTRALHOUSE', | ||
'BONCESPACE', | ||
'TRIBESCOWORK', | ||
'POWSPACE', | ||
'DEHOUSE', | ||
'WORKIN', | ||
'HOUSEOFBLOCKCHAIN', | ||
'SUBWORK', | ||
'POOLSIDE', | ||
'CAHOOTCOWORK', | ||
'NIFTYCLUB', | ||
'SPACESHACK', | ||
] |
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.
Consider moving promo codes to a secure configuration
Hardcoding promo codes in the client-side code exposes them to potential abuse. Consider:
- Moving promo codes to a server-side configuration
- Implementing server-side validation
- Adding expiration dates and usage limits
Would you like assistance in implementing a more secure promo code validation system?
const handlePromoCodeSubmit = () => { | ||
const normalizedCode = promoCheckerState.code.trim().toUpperCase() | ||
|
||
if (VALID_PROMO_CODES.includes(normalizedCode)) { | ||
setPromoCheckerState((prev) => ({ | ||
...prev, | ||
error: '', | ||
isApplied: true, | ||
})) | ||
onPromoCodeApplied(normalizedCode) | ||
} else if (normalizedCode === '') { | ||
setPromoCheckerState((prev) => ({ | ||
...prev, | ||
error: 'Please enter a promo code', | ||
isApplied: false, | ||
})) | ||
onPromoCodeApplied(null) | ||
} else { | ||
{ | ||
setPromoCheckerState((prev) => ({ | ||
...prev, | ||
error: 'Invalid promo code', | ||
isApplied: false, | ||
})) | ||
onPromoCodeApplied(null) | ||
} | ||
} | ||
} |
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.
🛠️ Refactor suggestion
Improve promo code validation and error handling
- Remove redundant block statement at lines 40-48
- Add input sanitization
- Handle edge cases (special characters, max length)
Apply these improvements:
const handlePromoCodeSubmit = () => {
const normalizedCode = promoCheckerState.code.trim().toUpperCase()
+ const MAX_LENGTH = 20
+
+ if (normalizedCode.length > MAX_LENGTH) {
+ setPromoCheckerState((prev) => ({
+ ...prev,
+ error: 'Promo code is too long',
+ isApplied: false,
+ }))
+ onPromoCodeApplied(null)
+ return
+ }
+
+ if (!/^[A-Z0-9]+$/.test(normalizedCode)) {
+ setPromoCheckerState((prev) => ({
+ ...prev,
+ error: 'Invalid characters in promo code',
+ isApplied: false,
+ }))
+ onPromoCodeApplied(null)
+ return
+ }
if (VALID_PROMO_CODES.includes(normalizedCode)) {
setPromoCheckerState((prev) => ({
...prev,
error: '',
isApplied: true,
}))
onPromoCodeApplied(normalizedCode)
} else if (normalizedCode === '') {
setPromoCheckerState((prev) => ({
...prev,
error: 'Please enter a promo code',
isApplied: false,
}))
onPromoCodeApplied(null)
} else {
- {
setPromoCheckerState((prev) => ({
...prev,
error: 'Invalid promo code',
isApplied: false,
}))
onPromoCodeApplied(null)
- }
}
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const handlePromoCodeSubmit = () => { | |
const normalizedCode = promoCheckerState.code.trim().toUpperCase() | |
if (VALID_PROMO_CODES.includes(normalizedCode)) { | |
setPromoCheckerState((prev) => ({ | |
...prev, | |
error: '', | |
isApplied: true, | |
})) | |
onPromoCodeApplied(normalizedCode) | |
} else if (normalizedCode === '') { | |
setPromoCheckerState((prev) => ({ | |
...prev, | |
error: 'Please enter a promo code', | |
isApplied: false, | |
})) | |
onPromoCodeApplied(null) | |
} else { | |
{ | |
setPromoCheckerState((prev) => ({ | |
...prev, | |
error: 'Invalid promo code', | |
isApplied: false, | |
})) | |
onPromoCodeApplied(null) | |
} | |
} | |
} | |
const handlePromoCodeSubmit = () => { | |
const normalizedCode = promoCheckerState.code.trim().toUpperCase() | |
const MAX_LENGTH = 20 | |
if (normalizedCode.length > MAX_LENGTH) { | |
setPromoCheckerState((prev) => ({ | |
...prev, | |
error: 'Promo code is too long', | |
isApplied: false, | |
})) | |
onPromoCodeApplied(null) | |
return | |
} | |
if (!/^[A-Z0-9]+$/.test(normalizedCode)) { | |
setPromoCheckerState((prev) => ({ | |
...prev, | |
error: 'Invalid characters in promo code', | |
isApplied: false, | |
})) | |
onPromoCodeApplied(null) | |
return | |
} | |
if (VALID_PROMO_CODES.includes(normalizedCode)) { | |
setPromoCheckerState((prev) => ({ | |
...prev, | |
error: '', | |
isApplied: true, | |
})) | |
onPromoCodeApplied(normalizedCode) | |
} else if (normalizedCode === '') { | |
setPromoCheckerState((prev) => ({ | |
...prev, | |
error: 'Please enter a promo code', | |
isApplied: false, | |
})) | |
onPromoCodeApplied(null) | |
} else { | |
setPromoCheckerState((prev) => ({ | |
...prev, | |
error: 'Invalid promo code', | |
isApplied: false, | |
})) | |
onPromoCodeApplied(null) | |
} | |
} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 40-47: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
{appliedPromoCode | ||
? offrampType === _consts.OfframpType.CASHOUT | ||
? utils.formatTokenAmount(parseFloat(usdValue ?? '')) | ||
: tokenPrice && | ||
claimLinkData && | ||
utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount)) | ||
: accountType === 'iban' | ||
? offrampType == _consts.OfframpType.CASHOUT | ||
? utils.formatTokenAmount(parseFloat(usdValue ?? '') - 1) | ||
: tokenPrice && | ||
claimLinkData && | ||
utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount) - 1) | ||
: offrampType == _consts.OfframpType.CASHOUT | ||
? utils.formatTokenAmount(parseFloat(usdValue ?? '') - 0.5) | ||
: tokenPrice && | ||
claimLinkData && | ||
utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount) - 1) | ||
: offrampType == _consts.OfframpType.CASHOUT | ||
? utils.formatTokenAmount(parseFloat(usdValue ?? '') - 0.5) | ||
: tokenPrice && | ||
claimLinkData && | ||
utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount) - 0.5)} | ||
utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount) - 0.5)} |
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.
🛠️ Refactor suggestion
Simplify complex calculations with helper functions
The nested ternary expressions make the code difficult to read and maintain. Consider extracting the logic into helper functions.
+const calculateAmount = (
+ appliedPromoCode: string | null | undefined,
+ offrampType: _consts.OfframpType,
+ accountType: string | undefined,
+ usdValue: string | undefined,
+ tokenPrice: number | undefined,
+ claimLinkData: any
+) => {
+ if (!usdValue && offrampType === _consts.OfframpType.CASHOUT) return '0'
+ if (!tokenPrice || !claimLinkData) return '0'
+
+ const baseAmount =
+ offrampType === _consts.OfframpType.CASHOUT
+ ? parseFloat(usdValue ?? '')
+ : tokenPrice * parseFloat(claimLinkData.tokenAmount)
+
+ if (appliedPromoCode) return utils.formatTokenAmount(baseAmount)
+
+ const fee = accountType === 'iban' ? 1 : 0.5
+ return utils.formatTokenAmount(baseAmount - fee)
+}
-{appliedPromoCode
- ? offrampType === _consts.OfframpType.CASHOUT
- ? utils.formatTokenAmount(parseFloat(usdValue ?? ''))
- : tokenPrice &&
- claimLinkData &&
- utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount))
- : accountType === 'iban'
- ? offrampType == _consts.OfframpType.CASHOUT
- ? utils.formatTokenAmount(parseFloat(usdValue ?? '') - 1)
- : tokenPrice &&
- claimLinkData &&
- utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount) - 1)
- : offrampType == _consts.OfframpType.CASHOUT
- ? utils.formatTokenAmount(parseFloat(usdValue ?? '') - 0.5)
- : tokenPrice &&
- claimLinkData &&
- utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount) - 0.5)}
+{calculateAmount(appliedPromoCode, offrampType, accountType, usdValue, tokenPrice, claimLinkData)}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
{appliedPromoCode | |
? offrampType === _consts.OfframpType.CASHOUT | |
? utils.formatTokenAmount(parseFloat(usdValue ?? '')) | |
: tokenPrice && | |
claimLinkData && | |
utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount)) | |
: accountType === 'iban' | |
? offrampType == _consts.OfframpType.CASHOUT | |
? utils.formatTokenAmount(parseFloat(usdValue ?? '') - 1) | |
: tokenPrice && | |
claimLinkData && | |
utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount) - 1) | |
: offrampType == _consts.OfframpType.CASHOUT | |
? utils.formatTokenAmount(parseFloat(usdValue ?? '') - 0.5) | |
: tokenPrice && | |
claimLinkData && | |
utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount) - 1) | |
: offrampType == _consts.OfframpType.CASHOUT | |
? utils.formatTokenAmount(parseFloat(usdValue ?? '') - 0.5) | |
: tokenPrice && | |
claimLinkData && | |
utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount) - 0.5)} | |
utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount) - 0.5)} | |
{calculateAmount(appliedPromoCode, offrampType, accountType, usdValue, tokenPrice, claimLinkData)} |
${/* if promo code is applied, show full amount without fee deduction */} | ||
{appliedPromoCode | ||
? offrampType == OfframpType.CASHOUT | ||
? utils.formatTokenAmount(parseFloat(usdValue ?? tokenValue ?? '') - 1) | ||
? utils.formatTokenAmount(parseFloat(usdValue ?? tokenValue ?? '')) | ||
: tokenPrice && | ||
claimLinkData && | ||
utils.formatTokenAmount( | ||
tokenPrice * parseFloat(claimLinkData.tokenAmount) - 1 | ||
tokenPrice * parseFloat(claimLinkData.tokenAmount) | ||
) | ||
: offrampType == OfframpType.CASHOUT | ||
? utils.formatTokenAmount(parseFloat(usdValue ?? '') - 0.5) | ||
: tokenPrice && | ||
claimLinkData && | ||
utils.formatTokenAmount( | ||
tokenPrice * parseFloat(claimLinkData.tokenAmount) - 0.5 | ||
)} | ||
<MoreInfo | ||
text={ | ||
user?.accounts.find( | ||
: // if no promo code, apply fee deduction based on account type | ||
user?.accounts.find( | ||
(account) => | ||
account.account_identifier.replaceAll(/\s/g, '').toLowerCase() === | ||
offrampForm.recipient.replaceAll(/\s/g, '').toLowerCase() | ||
)?.account_type === 'iban' | ||
? 'For SEPA transactions a fee of $1 is charged. For ACH transactions a fee of $0.50 is charged. This will be deducted of the amount you will receive.' | ||
: 'For ACH transactions a fee of $0.50 is charged. For SEPA transactions a fee of $1 is charged. This will be deducted of the amount you will receive.' | ||
? offrampType == OfframpType.CASHOUT | ||
? utils.formatTokenAmount(parseFloat(usdValue ?? tokenValue ?? '') - 1) | ||
: tokenPrice && | ||
claimLinkData && | ||
utils.formatTokenAmount( | ||
tokenPrice * parseFloat(claimLinkData.tokenAmount) - 1 | ||
) | ||
: offrampType == OfframpType.CASHOUT | ||
? utils.formatTokenAmount(parseFloat(usdValue ?? '') - 0.5) | ||
: tokenPrice && | ||
claimLinkData && | ||
utils.formatTokenAmount( | ||
tokenPrice * parseFloat(claimLinkData.tokenAmount) - 0.5 | ||
)} | ||
<MoreInfo |
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.
🛠️ Refactor suggestion
Simplify complex amount calculation logic.
The nested ternary expressions make the code hard to read and maintain. Consider extracting the logic into helper functions.
+const calculateAmount = (baseAmount: number, accountType: string, hasPromoCode: boolean) => {
+ if (hasPromoCode) return baseAmount;
+ const fee = accountType === 'iban' ? 1 : 0.5;
+ return baseAmount - fee;
+};
+
+const getBaseAmount = (offrampType: OfframpType, usdValue: string, tokenPrice?: number, claimLinkData?: any) => {
+ if (offrampType === OfframpType.CASHOUT) {
+ return parseFloat(usdValue ?? '0');
+ }
+ return tokenPrice && claimLinkData ? tokenPrice * parseFloat(claimLinkData.tokenAmount) : 0;
+};
-{/* complex nested ternaries */}
+{utils.formatTokenAmount(calculateAmount(
+ getBaseAmount(offrampType, usdValue ?? '', tokenPrice, claimLinkData),
+ getAccountType(user?.accounts, offrampForm.recipient),
+ !!appliedPromoCode
+))}
Committable suggestion skipped: line range outside the PR's diff.
{appliedPromoCode | ||
? '$0' | ||
: user?.accounts.find( | ||
(account) => | ||
account.account_identifier.replaceAll(/\s/g, '').toLowerCase() === | ||
offrampForm.recipient.replaceAll(/\s/g, '').toLowerCase() | ||
)?.account_type === 'iban' | ||
? '$1' | ||
: '$0.50'} | ||
<span className="inline-flex items-center"> | ||
<MoreInfo | ||
text={`For ${ | ||
user?.accounts.find( | ||
(account) => | ||
account.account_identifier | ||
.replaceAll(/\s/g, '') | ||
.toLowerCase() === | ||
offrampForm.recipient.replaceAll(/\s/g, '').toLowerCase() | ||
)?.account_type === 'iban' | ||
? 'SEPA' | ||
: 'ACH' | ||
} transactions a fee of ${ | ||
user?.accounts.find( | ||
(account) => | ||
account.account_identifier | ||
.replaceAll(/\s/g, '') | ||
.toLowerCase() === | ||
offrampForm.recipient.replaceAll(/\s/g, '').toLowerCase() | ||
)?.account_type === 'iban' | ||
? '$1' | ||
: '$0.50' | ||
} is charged.`} | ||
text={ | ||
appliedPromoCode | ||
? 'Fees waived with promo code!' | ||
: `For ${ | ||
user?.accounts.find( | ||
(account) => | ||
account.account_identifier | ||
.replaceAll(/\s/g, '') | ||
.toLowerCase() === | ||
offrampForm.recipient | ||
.replaceAll(/\s/g, '') | ||
.toLowerCase() | ||
)?.account_type === 'iban' | ||
? 'SEPA' | ||
: 'ACH' | ||
} transactions a fee of ${ | ||
user?.accounts.find( | ||
(account) => | ||
account.account_identifier | ||
.replaceAll(/\s/g, '') | ||
.toLowerCase() === | ||
offrampForm.recipient | ||
.replaceAll(/\s/g, '') | ||
.toLowerCase() | ||
)?.account_type === 'iban' | ||
? '$1' | ||
: '$0.50' | ||
} is charged.` | ||
} |
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.
🛠️ Refactor suggestion
Refactor fee display logic to reduce duplication.
The fee display logic contains repeated account type checks. Consider extracting this logic into a helper function.
+const getFeeAmount = (accountType: string) => accountType === 'iban' ? 1 : 0.5;
+const getAccountType = (accounts: any[], recipient: string) =>
+ accounts.find(account =>
+ account.account_identifier.replaceAll(/\s/g, '').toLowerCase() ===
+ recipient.replaceAll(/\s/g, '').toLowerCase()
+ )?.account_type;
{appliedPromoCode
? '$0'
- : user?.accounts.find(
- (account) =>
- account.account_identifier.replaceAll(/\s/g, '').toLowerCase() ===
- offrampForm.recipient.replaceAll(/\s/g, '').toLowerCase()
- )?.account_type === 'iban'
- ? '$1'
- : '$0.50'
+ : `$${getFeeAmount(getAccountType(user?.accounts, offrampForm.recipient))}`
}
Committable suggestion skipped: line range outside the PR's diff.
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
♻️ Duplicate comments (1)
src/components/Offramp/PromoCodeChecker.tsx (1)
22-49
: 🛠️ Refactor suggestionEnhance promo code validation and error handling
The current implementation needs improvements in validation and code structure.
Apply these improvements:
const handlePromoCodeSubmit = () => { const normalizedCode = promoCheckerState.code.trim().toUpperCase() + const MAX_LENGTH = 20 + + if (normalizedCode.length > MAX_LENGTH) { + setPromoCheckerState((prev) => ({ + ...prev, + error: 'Promo code is too long', + isApplied: false, + })) + onPromoCodeApplied(null) + return + } + + if (!/^[A-Z0-9]+$/.test(normalizedCode)) { + setPromoCheckerState((prev) => ({ + ...prev, + error: 'Invalid characters in promo code', + isApplied: false, + })) + onPromoCodeApplied(null) + return + } if (VALID_PROMO_CODES.includes(normalizedCode)) { setPromoCheckerState((prev) => ({ ...prev, error: '', isApplied: true, })) onPromoCodeApplied(normalizedCode) } else if (normalizedCode === '') { setPromoCheckerState((prev) => ({ ...prev, error: 'Please enter a promo code', isApplied: false, })) onPromoCodeApplied(null) } else { - { - setPromoCheckerState((prev) => ({ - ...prev, - error: 'Invalid promo code', - isApplied: false, - })) - onPromoCodeApplied(null) - } + setPromoCheckerState((prev) => ({ + ...prev, + error: 'Invalid promo code', + isApplied: false, + })) + onPromoCodeApplied(null) } }🧰 Tools
🪛 Biome (1.9.4)
[error] 40-47: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
🧹 Nitpick comments (2)
src/components/Offramp/PromoCodeChecker.tsx (2)
51-57
: Consider memoizing the toggle handlerThe handler function could be memoized using useCallback to prevent unnecessary re-renders.
+import React, { useState, useCallback } from 'react' -const handleExpandToggle = () => { +const handleExpandToggle = useCallback(() => { setPromoCheckerState((prev) => ({ ...prev, isExpanded: !prev.isExpanded, ...(prev.isExpanded && !prev.isApplied ? { code: '', error: '' } : {}), })) -} +}, [])
92-104
: Enhance input accessibilityThe promo code input field should have proper ARIA attributes and labels for better accessibility.
<input type="text" value={promoCheckerState.code} onChange={(e) => setPromoCheckerState((prev) => ({ ...prev, code: e.target.value }))} placeholder="Enter promo code" + aria-label="Promo code input" + aria-invalid={!!promoCheckerState.error} + aria-describedby="promo-code-error" className={`custom-input h-8 px-3 text-xs transition-all duration-300 ${ promoCheckerState.error ? 'border border-red' : '' }`} /> <button onClick={handlePromoCodeSubmit} + aria-label="Apply promo code" className="btn-purple-2 btn-small"> Apply </button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Offramp/PromoCodeChecker.tsx
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Offramp/PromoCodeChecker.tsx
[error] 40-47: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
🔇 Additional comments (1)
src/components/Offramp/PromoCodeChecker.tsx (1)
3-3
: Verify VALID_PROMO_CODES implementation
Let's ensure the constant is properly defined and maintained.
✅ Verification successful
VALID_PROMO_CODES constant is properly defined and used
The constant is well-defined in Offramp.consts.ts
as an array of valid promo codes and is correctly imported and used in PromoCodeChecker.tsx
. The implementation is clean and maintainable.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for VALID_PROMO_CODES definition and usage
rg "VALID_PROMO_CODES" --type typescript
Length of output: 75
Script:
#!/bin/bash
# Search for VALID_PROMO_CODES definition and usage with ripgrep
rg "VALID_PROMO_CODES"
Length of output: 297
Script:
#!/bin/bash
# Find the Offramp.consts file
fd "Offramp.consts"
Length of output: 60
Script:
#!/bin/bash
# If we find the file, let's look at its content
fd "Offramp.consts" --exec cat {}
Length of output: 3056
Summary by CodeRabbit
Release Notes
New Features
useAppKit
across multiple components.Bug Fixes
Chores