-
Notifications
You must be signed in to change notification settings - Fork 13
[TASK-7535] feat: add promo-code checker #577
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces a promo code feature for the cashout process, enhancing multiple files. It allows users to apply promotional codes during cashout, impacting the API route, utility functions, and frontend components. A new Changes
Possibly related PRs
Suggested reviewers
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: 4
🧹 Nitpick comments (5)
src/components/Offramp/PromoCodeChecker.tsx (2)
40-48
: Remove redundant block statementThe extra block statement can be safely removed.
} 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)
98-100
: Add loading state and prevent double submissionThe Apply button should show a loading state during validation and prevent double submissions.
-<button onClick={handlePromoCodeSubmit} className="btn-purple-2 btn-small"> +<button + onClick={handlePromoCodeSubmit} + disabled={isLoading || !promoCheckerState.code.trim()} + className={`btn-purple-2 btn-small ${isLoading ? 'opacity-50 cursor-not-allowed' : ''}`} +> - Apply + {isLoading ? 'Validating...' : 'Apply'} </button>src/app/api/peanut/submit-cashout-link/route.ts (1)
Line range hint
38-47
: Improve error handling for API responsesAdd specific error handling for promo code related errors from the API.
if (!response.ok) { + const errorData = await response.json().catch(() => ({})) + + // Handle specific error cases + if (errorData.code === 'INVALID_PROMO_CODE') { + return new NextResponse('Invalid or expired promo code', { status: 400 }) + } + throw new Error(`HTTP error! status: ${response.status}`) } const data = await response.json() return NextResponse.json(data)src/components/Offramp/Confirm.view.tsx (2)
100-104
: LGTM: Simple and effective promo code handlerThe handler function is straightforward and includes a TODO for tracking promo code usage.
Consider implementing the promo code usage tracking before shipping to production.
815-844
: Refactor amount calculation to improve readabilityThe amount calculation logic is complex and hard to read. Consider extracting it into a helper function.
+const calculateFinalAmount = ( + baseAmount: number, + accountType: string, + appliedPromoCode: string | null +) => { + if (appliedPromoCode) return baseAmount; + const fee = accountType === 'iban' ? 1 : 0.5; + return baseAmount - fee; +}; -${appliedPromoCode - ? offrampType == OfframpType.CASHOUT - ? utils.formatTokenAmount(parseFloat(usdValue ?? tokenValue ?? '')) - : tokenPrice && - claimLinkData && - utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount)) - : user?.accounts.find(...)?.account_type === 'iban' - ? // Complex nested logic - : // More complex nested logic -} +${utils.formatTokenAmount( + calculateFinalAmount( + offrampType === OfframpType.CASHOUT + ? parseFloat(usdValue ?? tokenValue ?? '') + : tokenPrice && claimLinkData + ? tokenPrice * parseFloat(claimLinkData.tokenAmount) + : 0, + user?.accounts.find(...)?.account_type ?? '', + appliedPromoCode + ) +)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/app/api/peanut/submit-cashout-link/route.ts
(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/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 (3)
src/utils/cashout.utils.ts (1)
526-526
: LGTM: Clean addition of promo code parameter
The optional promoCode parameter is correctly added to the function signature and properly included in the request body.
Also applies to: 546-546
src/components/Offramp/Confirm.view.tsx (1)
73-76
: LGTM: Clean state management implementation
The promo code state is properly managed using useState hook.
src/components/Global/Icon/index.tsx (1)
205-206
: LGTM: Clean addition of new icons
The new icons are properly added to the icons object with correct SVG paths.
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.
Security concern: Promo codes should not be hardcoded in frontend code
Moving promo code validation to the server side is strongly recommended for several reasons:
- Current implementation exposes all valid codes to users
- No way to manage expiration dates or usage limits
- Codes can be easily extracted from the client-side bundle
Consider implementing a secure promo code validation API endpoint that:
- Stores codes securely in a database
- Validates codes server-side
- Tracks usage limits and expiration dates
- Returns only the validation result, not the full list of codes
Would you like me to help design a more secure promo code validation system?
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.
very true
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.
@jjramirezn, ¡gracias por la confirmación! Si deseas abordar esto en un futuro PR, podemos crear una nueva tarea o abrir un nuevo issue para implementar la validación segura de los códigos promocionales en el servidor. Estoy aquí para ayudarte con el diseño si así lo decides.
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.
@jjramirezn @kushagrasarathe we considered this, for current implementation doing it hackily hardcoded is perfectly fine. If we run more campaigns in future, we'll do it properly
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
<input | ||
type="text" | ||
value={promoCheckerState.code} | ||
onChange={(e) => setPromoCheckerState((prev) => ({ ...prev, code: e.target.value }))} | ||
placeholder="Enter promo code" | ||
className={`custom-input h-8 px-3 text-xs transition-all duration-300 ${ | ||
promoCheckerState.error ? 'border border-red' : '' | ||
}`} | ||
/> |
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
Enhance input field accessibility and validation
The promo code input field needs accessibility improvements and input validation.
<input
type="text"
+ aria-label="Promo code input"
+ maxLength={20}
+ pattern="[A-Za-z0-9]+"
value={promoCheckerState.code}
onChange={(e) => setPromoCheckerState((prev) => ({ ...prev, code: e.target.value }))}
placeholder="Enter promo code"
className={`custom-input h-8 px-3 text-xs transition-all duration-300 ${
promoCheckerState.error ? 'border border-red' : ''
}`}
/>
📝 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.
<input | |
type="text" | |
value={promoCheckerState.code} | |
onChange={(e) => setPromoCheckerState((prev) => ({ ...prev, code: e.target.value }))} | |
placeholder="Enter promo code" | |
className={`custom-input h-8 px-3 text-xs transition-all duration-300 ${ | |
promoCheckerState.error ? 'border border-red' : '' | |
}`} | |
/> | |
<input | |
type="text" | |
aria-label="Promo code input" | |
maxLength={20} | |
pattern="[A-Za-z0-9]+" | |
value={promoCheckerState.code} | |
onChange={(e) => setPromoCheckerState((prev) => ({ ...prev, code: e.target.value }))} | |
placeholder="Enter promo code" | |
className={`custom-input h-8 px-3 text-xs transition-all duration-300 ${ | |
promoCheckerState.error ? 'border border-red' : '' | |
}`} | |
/> |
externalAccountId, | ||
chainId, | ||
tokenName, | ||
promoCode, |
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
Add promoCode validation and error handling
The route should validate the promoCode and handle promo-specific errors from the API.
const {
pubKey,
bridgeCustomerId,
liquidationAddressId,
cashoutTransactionHash,
externalAccountId,
chainId,
tokenName,
promoCode,
} = await request.json()
+// Validate promoCode if provided
+if (promoCode && typeof promoCode !== 'string') {
+ return new NextResponse('Invalid promo code format', { status: 400 })
+}
const apiKey = process.env.PEANUT_API_KEY!
const response = await fetch(`${consts.PEANUT_API_URL}/cashouts`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'api-key': apiKey,
},
body: JSON.stringify({
bridgeCustomerId,
liquidationAddressId,
cashoutTransactionHash,
externalAccountId,
chainId,
tokenName,
pubKey,
- promoCode,
+ ...(promoCode && { promoCode }),
}),
})
Also applies to: 35-35
{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 calculation logic to reduce duplication
The fee calculation logic is duplicated multiple times. Consider extracting it into a helper function.
+const getFee = (accountType: string, appliedPromoCode: string | null) => {
+ if (appliedPromoCode) return 0;
+ return accountType === 'iban' ? 1 : 0.50;
+};
+const getFeeText = (accountType: string) => {
+ return accountType === 'iban' ? 'SEPA' : 'ACH';
+};
-{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'}
+{`$${getFee(
+ user?.accounts.find(
+ (account) =>
+ account.account_identifier.replaceAll(/\s/g, '').toLowerCase() ===
+ offrampForm.recipient.replaceAll(/\s/g, '').toLowerCase()
+ )?.account_type ?? '',
+ appliedPromoCode
+)}`}
Committable suggestion skipped: line range outside the PR's diff.
…into feat/promo-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: 1
🧹 Nitpick comments (2)
src/components/Offramp/Confirm.view.tsx (2)
102-102
: Address the TODO comment for tracking promo code usage.Consider implementing analytics tracking for promo code usage to measure the effectiveness of promotional campaigns.
Would you like me to help implement the analytics tracking code or create a GitHub issue to track this task?
450-451
: Consider separating promoCode and tracking parameters.Using the same value for both
promoCode
andtrackParam
might limit future tracking capabilities. Consider using a more specific tracking parameter that could include additional context.-promoCode: appliedPromoCode || '', -trackParam: appliedPromoCode || '', +promoCode: appliedPromoCode || '', +trackParam: appliedPromoCode ? `promo_${appliedPromoCode}` : '',
📜 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 (3)
src/app/api/peanut/submit-cashout-link/route.ts
(2 hunks)src/components/Offramp/Confirm.view.tsx
(6 hunks)src/utils/cashout.utils.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/api/peanut/submit-cashout-link/route.ts
- src/utils/cashout.utils.ts
🔇 Additional comments (2)
src/components/Offramp/Confirm.view.tsx (2)
863-864
: LGTM!
The PromoCodeChecker component is properly integrated with the correct props.
737-776
: 🛠️ Refactor suggestion
Refactor fee display logic to improve maintainability.
The current implementation has several maintainability issues:
- Complex nested ternary expressions
- Duplicated account type checking logic
- Repeated string manipulation for account comparison
Consider extracting the common logic into helper functions:
+const getAccountType = (accounts: any[], recipient: string) => {
+ return accounts?.find(
+ (account) =>
+ account.account_identifier.replaceAll(/\s/g, '').toLowerCase() ===
+ recipient.replaceAll(/\s/g, '').toLowerCase()
+ )?.account_type;
+};
+const getFeeAmount = (accountType: string, hasPromoCode: boolean) => {
+ if (hasPromoCode) return 0;
+ return accountType === 'iban' ? 1 : 0.50;
+};
+const getFeeText = (accountType: string, hasPromoCode: boolean) => {
+ if (hasPromoCode) return 'Fees waived with promo code!';
+ return `For ${accountType === 'iban' ? 'SEPA' : 'ACH'} transactions a fee of $${
+ accountType === 'iban' ? '1' : '0.50'
+ } is charged.`;
+};
Then simplify the JSX:
-{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), !!appliedPromoCode)}`}
Likely invalid or redundant comment.
${/* 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 | ||
text={ | ||
appliedPromoCode | ||
? 'Fees waived with promo code!' | ||
: 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.' |
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 total amount calculation logic.
The amount calculation logic has similar maintainability issues as the fee display:
- Complex nested conditionals
- Duplicated account type checking
- Repeated calculations
Consider extracting the calculation logic into a helper function:
+const calculateFinalAmount = (
+ 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 | undefined,
+ tokenValue: string | undefined,
+ tokenPrice: number | undefined,
+ claimLinkData: any,
+) => {
+ if (offrampType === OfframpType.CASHOUT) {
+ return parseFloat(usdValue ?? tokenValue ?? '0');
+ }
+ return tokenPrice && claimLinkData
+ ? tokenPrice * parseFloat(claimLinkData.tokenAmount)
+ : 0;
+};
Then simplify the JSX:
-${appliedPromoCode
- ? offrampType == OfframpType.CASHOUT
- ? utils.formatTokenAmount(parseFloat(usdValue ?? tokenValue ?? ''))
- : tokenPrice &&
- claimLinkData &&
- utils.formatTokenAmount(tokenPrice * parseFloat(claimLinkData.tokenAmount))
- : // if no promo code, apply fee deduction...
+${utils.formatTokenAmount(
+ calculateFinalAmount(
+ getBaseAmount(offrampType, usdValue, tokenValue, tokenPrice, claimLinkData),
+ getAccountType(user?.accounts, offrampForm.recipient),
+ !!appliedPromoCode
+ )
+)}
Committable suggestion skipped: line range outside the PR's diff.
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.
@jjramirezn @kushagrasarathe we considered this, for current implementation doing it hackily hardcoded is perfectly fine. If we run more campaigns in future, we'll do it properly
Overall looks good, but blocking issue: Need to update Fee in success view too, not just confirm view |
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: 3
♻️ Duplicate comments (1)
src/components/Offramp/PromoCodeChecker.tsx (1)
91-99
: 🛠️ Refactor suggestionEnhance input field accessibility and validation
The promo code input field needs accessibility improvements and input validation.
<input type="text" + aria-label="Promo code input" + maxLength={20} + pattern="[A-Za-z0-9]+" value={promoCheckerState.code} onChange={(e) => setPromoCheckerState((prev) => ({ ...prev, code: e.target.value }))} placeholder="Enter promo code" className={`custom-input h-8 px-3 text-xs transition-all duration-300 ${ promoCheckerState.error ? 'border border-red' : '' }`} />
🧹 Nitpick comments (1)
src/components/Offramp/Success.view.tsx (1)
92-96
: Reduce text duplication in fee explanationThe fee explanation text is duplicated with only the order of SEPA and ACH mentions swapped. Consider using a single, consistent message.
- 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={ + appliedPromoCode + ? `Fees waived with promo code ${appliedPromoCode}` + : 'For SEPA transactions a fee of $1 is charged and for ACH transactions a fee of $0.50 is charged. This will be deducted from the amount you will receive.' + }
📜 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 (5)
src/components/Cashout/Cashout.consts.ts
(3 hunks)src/components/Claim/Link/FlowManager.tsx
(2 hunks)src/components/Offramp/Offramp.consts.ts
(1 hunks)src/components/Offramp/PromoCodeChecker.tsx
(1 hunks)src/components/Offramp/Success.view.tsx
(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 (7)
src/components/Claim/Link/FlowManager.tsx (1)
22-22
: LGTM! Verify prop propagation to child components.
The type definition and prop handling for appliedPromoCode
are implemented correctly.
Let's verify that child components handle the appliedPromoCode
prop correctly:
Also applies to: 36-37
✅ Verification successful
Props are correctly propagated and handled in child components
The verification shows that appliedPromoCode
is properly integrated throughout the component hierarchy:
- In
FlowManager.tsx
, the prop is correctly passed down to Offramp components Offramp.consts.ts
properly defines the type interface- Child components handle the prop appropriately:
Success.view.tsx
uses it for fee calculations and display logicConfirm.view.tsx
manages it via state and uses it for API calls and UI rendering
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if child components properly type and handle the appliedPromoCode prop
# Look for prop usage in Offramp components
ast-grep --pattern 'interface $interface {
$$$
appliedPromoCode?: string
$$$
}'
Length of output: 87
Script:
#!/bin/bash
# Let's try a different approach to find appliedPromoCode prop usage
# First, let's see the actual file content to understand the component hierarchy
cat src/components/Claim/Link/FlowManager.tsx
# Then search for files that might use appliedPromoCode
rg -l "appliedPromoCode" src/
# And get more context around its usage
rg -A 3 -B 3 "appliedPromoCode" src/
Length of output: 11437
src/components/Cashout/Cashout.consts.ts (1)
6-6
: LGTM! Type safety improvements.
Good improvements to type safety:
- Replacing
any
with proper interface typing - Consistent prop types across components
Also applies to: 23-23, 68-69
src/components/Offramp/Offramp.consts.ts (2)
79-79
: LGTM! Interface update is consistent.
The appliedPromoCode
prop is consistently typed across components.
82-119
: Security: Hardcoded promo codes as discussed.
While hardcoding promo codes in the frontend is generally not recommended for security reasons, this implementation aligns with the team's current requirements as discussed in PR comments.
For future reference, when implementing a more robust solution, consider:
- Moving validation to the server
- Adding expiration dates
- Implementing usage limits
- Storing codes securely
Note: This comment is marked as duplicate as it was previously discussed in PR comments and approved by maintainer Hugo0 for the current implementation.
src/components/Offramp/PromoCodeChecker.tsx (2)
5-17
: LGTM! Well-structured state management
The PromoState interface and initial state are well-defined with clear type definitions and appropriate default values.
51-57
: LGTM! Clean toggle implementation
The expand toggle function properly manages state transitions and cleanup.
src/components/Offramp/Success.view.tsx (1)
50-58
: LGTM! Clear fee display logic
The fee display logic properly handles both promo code and regular scenarios with appropriate messaging.
{ | ||
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
Remove redundant block statement
The block statement here is unnecessary and can be simplified.
- {
- setPromoCheckerState((prev) => ({
- ...prev,
- error: 'Invalid promo code',
- isApplied: false,
- }))
- onPromoCodeApplied(null)
- }
+ 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.
{ | |
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)
{promoCheckerState.isApplied ? ( | ||
<p className="text-center text-sm text-green-600"> | ||
Promo code ${promoCheckerState.code} applied successfully! | ||
</p> |
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.
Fix string interpolation in success message
The success message has incorrect string interpolation syntax.
- Promo code ${promoCheckerState.code} applied successfully!
+ Promo code {promoCheckerState.code} applied successfully!
📝 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.
{promoCheckerState.isApplied ? ( | |
<p className="text-center text-sm text-green-600"> | |
Promo code ${promoCheckerState.code} applied successfully! | |
</p> | |
{promoCheckerState.isApplied ? ( | |
<p className="text-center text-sm text-green-600"> | |
Promo code {promoCheckerState.code} applied successfully! | |
</p> |
{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 amount calculation logic
The nested ternary expressions make the code hard to read and maintain. Consider extracting the calculation logic into a separate function.
+ const calculateAmount = () => {
+ const baseAmount = offrampType === _consts.OfframpType.CASHOUT
+ ? parseFloat(usdValue ?? '')
+ : tokenPrice && claimLinkData
+ ? tokenPrice * parseFloat(claimLinkData.tokenAmount)
+ : 0;
+
+ if (appliedPromoCode) return baseAmount;
+ return baseAmount - (accountType === 'iban' ? 1 : 0.5);
+ };
- {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)}
+ {utils.formatTokenAmount(calculateAmount())}
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: 1
🧹 Nitpick comments (3)
src/components/Offramp/PromoCodeChecker.tsx (3)
5-17
: Consider enhancing type safety for the promo code stateThe interface structure is good, but we can make it more robust with these improvements:
interface PromoState { isExpanded: boolean - code: string + code: string + normalizedCode: string // Add to track normalized version error: string isApplied: boolean } const INITIAL_STATE: PromoState = { isExpanded: false, code: '', + normalizedCode: '', error: '', isApplied: false, }
51-57
: Consider separating expand/collapse and reset logicThe state management in handleExpandToggle could be more explicit:
const handleExpandToggle = () => { + const shouldReset = promoCheckerState.isExpanded && !promoCheckerState.isApplied + setPromoCheckerState((prev) => ({ ...prev, isExpanded: !prev.isExpanded, - ...(prev.isExpanded && !prev.isApplied ? { code: '', error: '' } : {}), + ...(shouldReset && { + code: '', + normalizedCode: '', + error: '' + }) })) }
78-83
: Optimize transition performanceConsider using CSS transform instead of max-height for smoother animations:
<div className={` - overflow-hidden transition-all duration-300 ease-in-out - ${promoCheckerState.isExpanded ? 'max-h-40 opacity-100' : 'max-h-0 opacity-0'} + transform-origin: top; + transition: transform 300ms ease-in-out, opacity 300ms ease-in-out; + ${promoCheckerState.isExpanded + ? 'transform: scaleY(1); opacity: 1;' + : 'transform: scaleY(0); opacity: 0;' + } `} >
📜 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)
85-87
:
Fix string interpolation in success message
The success message needs proper JSX string interpolation:
<p className="text-center text-sm text-green-600">
- Promo code {promoCheckerState.code} applied successfully!
+ Promo code <strong>{promoCheckerState.normalizedCode}</strong> applied successfully!
</p>
Likely invalid or redundant comment.
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
The validation logic can be enhanced for better maintainability and security:
const handlePromoCodeSubmit = () => {
- const normalizedCode = promoCheckerState.code.trim().toUpperCase()
+ // Sanitize input: remove spaces and special characters
+ const normalizedCode = promoCheckerState.code
+ .trim()
+ .replace(/[^A-Za-z0-9]/g, '')
+ .toUpperCase()
+ const updateState = (error: string, isApplied: boolean) => {
+ setPromoCheckerState((prev) => ({
+ ...prev,
+ error,
+ isApplied,
+ normalizedCode
+ }))
+ onPromoCodeApplied(isApplied ? normalizedCode : null)
+ }
if (VALID_PROMO_CODES.includes(normalizedCode)) {
- setPromoCheckerState((prev) => ({
- ...prev,
- error: '',
- isApplied: true,
- }))
- onPromoCodeApplied(normalizedCode)
+ updateState('', true)
} else if (normalizedCode === '') {
- setPromoCheckerState((prev) => ({
- ...prev,
- error: 'Please enter a promo code',
- isApplied: false,
- }))
- onPromoCodeApplied(null)
+ updateState('Please enter a promo code', false)
} else {
- {
- setPromoCheckerState((prev) => ({
- ...prev,
- error: 'Invalid promo code',
- isApplied: false,
- }))
- onPromoCodeApplied(null)
- }
+ updateState('Invalid promo code', false)
}
}
📝 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 = () => { | |
// Sanitize input: remove spaces and special characters | |
const normalizedCode = promoCheckerState.code | |
.trim() | |
.replace(/[^A-Za-z0-9]/g, '') | |
.toUpperCase() | |
const updateState = (error: string, isApplied: boolean) => { | |
setPromoCheckerState((prev) => ({ | |
...prev, | |
error, | |
isApplied, | |
normalizedCode | |
})) | |
onPromoCodeApplied(isApplied ? normalizedCode : null) | |
} | |
if (VALID_PROMO_CODES.includes(normalizedCode)) { | |
updateState('', true) | |
} else if (normalizedCode === '') { | |
updateState('Please enter a promo code', false) | |
} else { | |
updateState('Invalid promo code', false) | |
} | |
} |
🧰 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)
total
amount andfee
amount if promo is appliedSummary by CodeRabbit
New Features
ticket
andchevron-up
.PromoCodeChecker
component for user-friendly promo code input.Bug Fixes
Chores