feat(aih): fixed coupon discount #578
Conversation
Created extensive test coverage for fixed amount discount feature: - Schema validation with mutual exclusivity - Price calculation with fixed/percentage discounts - Coupon selection logic and priority - Stripe integration with transient coupons - Edge cases and error handling 200+ tests following TDD approach. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…t coupons - Added support for amountDiscount in merchant coupon schema with validation for mutual exclusivity with percentageDiscount. - Updated pricing logic to handle both fixed amount and percentage discounts in price calculations. - Enhanced coupon application logic to determine the type of discount being applied. - Integrated new discount handling in Stripe checkout process, including transient coupon creation for fixed amount discounts. - Updated related types and schemas to accommodate new discount features. This commit enhances the flexibility of coupon offerings, allowing for both fixed and percentage discounts to be applied effectively.
…ixed-coupon-discount
- Mark Phase 1 shared components as completed - Update checklist with completed items 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…ghout pricing logic - Compare coupons by actual dollar amounts instead of percentages only - Add appliedFixedDiscount field to FormattedPrice type - Fix upgrade discount conversion from cents to dollars - Ensure consistent currency units across pricing calculations 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add unitPrice parameter to all determineCouponToApply test calls - Implement getPurchasesForUser mock in test adapter - Implement getMerchantCoupon mocks for type and percent lookup 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Show fixed discount amounts ($XX off) instead of percentage when applicable - Update both PriceDisplay and Pricing components - Add screen reader labels for fixed discounts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add CouponInput component for applying coupon codes - Add DiscountBadge component for showing discount type and amount - Add PriceBreakdown component for detailed price display - Support both fixed and percentage discount types 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds fixed (amount) discount support alongside percentage discounts across UI, schemas, DB, pricing core, Stripe integration, adapters, types, and tests; threads discountType/amountDiscount through validation and decision logic, compares discounts by monetary value using unitPrice, and exposes appliedDiscountType/appliedFixedDiscount in formatted prices. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant UI as Product Page (UI)
participant PR as Pricing API
participant CORE as Pricing Core
participant DB as Data/Adapters
participant ST as Stripe
U->>UI: View price / enter coupon
UI->>PR: getPrice/applyCoupon(productId, couponCode?, qty, upgradeFrom?)
PR->>CORE: formatPricesForProduct(...)
CORE->>DB: fetch product / price / merchant coupons
CORE->>CORE: determineCouponToApply(unitPrice, qty, coupons...)
note right of CORE #f9f5e7: merchantDiscount = amountDiscount/100 OR percentage × unitPrice\nCompare dollar amounts (PPP / bulk / merchant) and pick best
CORE->>CORE: getCalculatedPrice({ unitPrice, amountDiscount?, percentOfDiscount?, fixedDiscount? })
CORE-->>PR: FormattedPrice { appliedDiscountType, appliedFixedDiscount, ... }
PR-->>UI: Return price breakdown
UI-->>U: Render badge, breakdown, coupon state
alt Checkout with coupon
U->>UI: Checkout
UI->>PR: createCheckoutSession(...)
PR->>CORE: stripeCheckout(...)
alt appliedDiscountType == fixed
CORE->>ST: create amount_off coupon (transient) / include coupon in discounts
CORE-->>PR: metadata { discountType: fixed, discountAmount }
else appliedDiscountType == percentage
CORE->>ST: create promotion_code / include promotion_code
CORE-->>PR: metadata { discountType: percentage, percentOff }
end
PR-->>UI: Checkout URL
UI-->>U: Redirect to Stripe
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
zacjones93
added
the
WIP
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
apps/ai-hero/src/lib/coupons-query.ts (2)
20-22: Consider adding schema-level validation for discount type mutual exclusivity.The schema allows both
percentageDiscountandamountDiscountto be optional, relying on runtime validation increateCoupon(lines 187-189). However, themerchantCouponSchemainpackages/core/src/schemas/merchant-coupon-schema.tsenforces mutual exclusivity at the schema level using.refine(). For consistency and earlier error detection, consider adding similar validation here.Apply this diff to add schema-level validation:
const CouponInputSchema = z.object({ quantity: z.string(), maxUses: z.coerce.number(), expires: z.date().optional(), restrictedToProductId: z.string().optional(), discountType: z.enum(['percentage', 'fixed']).default('percentage'), percentageDiscount: z.string().optional(), amountDiscount: z.number().optional(), status: z.number().default(1), default: z.boolean().default(false), fields: z.record(z.any()).default({}), -}) +}).refine( + (data) => { + if (data.discountType === 'percentage') { + return data.percentageDiscount !== undefined + } + if (data.discountType === 'fixed') { + return data.amountDiscount !== undefined + } + return true + }, + { + message: 'percentageDiscount is required when discountType is "percentage", and amountDiscount is required when discountType is "fixed"', + }, +)
187-189: Clarify the error message to indicate mutual exclusivity.The error message states "must provide either percentageDiscount or amountDiscount" but doesn't explicitly convey that providing both is invalid. Consider making the mutual exclusivity constraint clearer.
Apply this diff:
} else { throw new Error( - 'Invalid discount configuration: must provide either percentageDiscount or amountDiscount', + 'Invalid discount configuration: must provide either percentageDiscount or amountDiscount, but not both', ) }plans/flat-discount-coupons-application-integration.md (1)
877-877: Add language specification to the fenced code block.The static analysis tool flagged a fenced code block without a language specifier. This improves syntax highlighting and readability.
Based on the context (appears to be Markdown documentation), apply this diff:
### Customization All components accept `className` prop for custom styling. ### Discount Logic Core discount logic lives in `@coursebuilder/core`. If you need app-specific behavior, extend in your app's tRPC layer. -``` +```typescript +// Example: Custom discount logic in tRPC layer +```packages/core/src/schemas/coupon-schema.ts (1)
14-18: Consider adding mutual exclusivity validation consistent with merchantCouponSchema.The
couponSchemaaddsamountDiscountbut doesn't enforce mutual exclusivity withpercentageDiscount, unlikemerchantCouponSchemawhich includes a.refine()check. While coupons reference merchant coupons (line 11), having consistent validation would prevent potential data integrity issues.Apply this diff to add validation:
-export const couponSchema = z.object({ +export const couponSchema = z.object({ id: z.string(), code: z.string().max(191).optional().nullable(), createdAt: z.date().nullable(), expires: z.date().nullable(), fields: z.record(z.any()).default({}), maxUses: z.number().int().default(-1), default: z.boolean().default(false), merchantCouponId: z.string().max(191).optional().nullable(), status: z.number().int().default(0), usedCount: z.number().int().default(0), percentageDiscount: z.coerce.number().refine((value) => { const decimalPlaces = value.toString().split('.')[1]?.length || 0 return decimalPlaces <= 2 }), amountDiscount: z.coerce.number().int().optional().nullable(), restrictedToProductId: z.string().max(191).optional().nullable(), bulkPurchases: z.array(z.any()).default([]), redeemedBulkCouponPurchases: z.array(z.any()).default([]), bulkPurchaseId: z.string().max(191).optional().nullable(), // TODO: remove organizationId: z.string().max(191).optional().nullable(), +}).refine( + (data) => { + const hasPercent = data.percentageDiscount > 0 + const hasAmount = data.amountDiscount !== null && data.amountDiscount !== undefined && data.amountDiscount > 0 + return !(hasPercent && hasAmount) + }, + { + message: 'Coupon cannot have both percentageDiscount and amountDiscount', + }, +) -})packages/core/test/merchant-coupon-schema.test.ts (1)
1-337: Excellent test coverage for schema validation.The test suite comprehensively covers:
- Fixed and percentage discount validation
- Mutual exclusivity between discount types
- Required vs optional fields
- Edge cases (zero, negative, null, undefined)
- String length constraints
Minor enhancement: Consider extracting the refinement error message check into a constant to ensure consistency with the schema:
const MUTUAL_EXCLUSIVITY_ERROR = 'Cannot have both percentageDiscount and amountDiscount with values > 0' // In test (line 122-124): expect(result.error.message).toContain(MUTUAL_EXCLUSIVITY_ERROR)This makes it easier to update if the message changes.
packages/core/src/lib/pricing/get-calculated-price.ts (1)
42-45: Consider logging when price clamping occurs.The code correctly clamps negative prices to 0, but this might indicate an issue with discount configuration (e.g., discount > product price).
Add logging to track when clamping happens:
// Clamp negative prices to 0 const clampedPrice = Math.max(0, finalPrice) + +if (finalPrice < 0) { + console.warn('Price clamped to 0', { + unitPrice, + quantity, + fixedDiscount, + amountDiscount, + percentOfDiscount, + finalPrice, + }) +} return Number(clampedPrice.toFixed(2))This helps identify misconfigured coupons or edge cases in production. Based on learnings
plans/flat-discount-coupons-decision-flow.md (1)
128-149: Add language identifiers to fenced code blocksMarkdownlint (MD040) flags these fences because they omit a language hint. Please add an explicit identifier (e.g., ```text) to this and the other generic code blocks in the doc so the lint step passes. Based on static analysis hints
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
apps/ai-hero/src/app/admin/coupons/_components/coupon-data-table.tsx(1 hunks)apps/ai-hero/src/app/admin/coupons/_components/coupon-generator-form.tsx(5 hunks)apps/ai-hero/src/lib/coupons-query.ts(4 hunks)packages/adapter-drizzle/src/lib/mysql/schemas/commerce/coupon.ts(1 hunks)packages/adapter-drizzle/src/lib/mysql/schemas/commerce/merchant-coupon.ts(1 hunks)packages/commerce-next/src/pricing/coupon-input.tsx(1 hunks)packages/commerce-next/src/pricing/discount-badge.tsx(1 hunks)packages/commerce-next/src/pricing/price-breakdown.tsx(1 hunks)packages/commerce-next/src/pricing/price-display.tsx(3 hunks)packages/commerce-next/src/pricing/pricing.tsx(3 hunks)packages/core/src/lib/actions/prices-formatted.ts(2 hunks)packages/core/src/lib/pricing/determine-coupon-to-apply.ts(11 hunks)packages/core/src/lib/pricing/format-prices-for-product.ts(4 hunks)packages/core/src/lib/pricing/get-calculated-price.ts(1 hunks)packages/core/src/lib/pricing/stripe-checkout.ts(5 hunks)packages/core/src/providers/stripe.ts(2 hunks)packages/core/src/schemas/coupon-schema.ts(1 hunks)packages/core/src/schemas/merchant-coupon-schema.ts(1 hunks)packages/core/src/schemas/stripe/checkout-session-metadata.ts(1 hunks)packages/core/src/types.ts(2 hunks)packages/core/test/determine-coupon-to-apply.test.ts(1 hunks)packages/core/test/format-prices-for-product.test.ts(1 hunks)packages/core/test/get-calculated-price.test.ts(1 hunks)packages/core/test/merchant-coupon-schema.test.ts(1 hunks)packages/core/test/pricing-test-fixtures.ts(1 hunks)packages/core/test/stripe-checkout-fixed-discounts.test.ts(1 hunks)plans/flat-discount-coupons-application-integration.md(1 hunks)plans/flat-discount-coupons-architecture.md(1 hunks)plans/flat-discount-coupons-code-changes.md(1 hunks)plans/flat-discount-coupons-decision-flow.md(1 hunks)plans/flat-discount-coupons-testing-plan.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Add JS Doc comments to functions and React components
Files:
packages/core/test/merchant-coupon-schema.test.tspackages/commerce-next/src/pricing/coupon-input.tsxpackages/commerce-next/src/pricing/price-display.tsxpackages/core/src/types.tspackages/commerce-next/src/pricing/pricing.tsxpackages/core/src/schemas/coupon-schema.tspackages/commerce-next/src/pricing/price-breakdown.tsxpackages/core/test/determine-coupon-to-apply.test.tspackages/core/src/schemas/stripe/checkout-session-metadata.tspackages/commerce-next/src/pricing/discount-badge.tsxpackages/core/test/format-prices-for-product.test.tspackages/core/test/pricing-test-fixtures.tspackages/core/src/lib/actions/prices-formatted.tspackages/core/test/get-calculated-price.test.tspackages/adapter-drizzle/src/lib/mysql/schemas/commerce/coupon.tspackages/core/src/providers/stripe.tsapps/ai-hero/src/app/admin/coupons/_components/coupon-generator-form.tsxpackages/core/src/lib/pricing/determine-coupon-to-apply.tspackages/core/src/lib/pricing/format-prices-for-product.tspackages/core/src/schemas/merchant-coupon-schema.tspackages/core/src/lib/pricing/get-calculated-price.tspackages/core/test/stripe-checkout-fixed-discounts.test.tsapps/ai-hero/src/lib/coupons-query.tspackages/adapter-drizzle/src/lib/mysql/schemas/commerce/merchant-coupon.tsapps/ai-hero/src/app/admin/coupons/_components/coupon-data-table.tsxpackages/core/src/lib/pricing/stripe-checkout.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use single quotes, no semicolons, tabs (width: 2), and an 80 character line limit for code formatting.
Organize imports in the following order: React → Next → 3rd party → internal.
Do NOT use Object.defineProperty(exports, ...) for re-exports. Use standard export patterns instead to avoid conflicts with framework internals.
Files:
packages/core/test/merchant-coupon-schema.test.tspackages/commerce-next/src/pricing/coupon-input.tsxpackages/commerce-next/src/pricing/price-display.tsxpackages/core/src/types.tspackages/commerce-next/src/pricing/pricing.tsxpackages/core/src/schemas/coupon-schema.tspackages/commerce-next/src/pricing/price-breakdown.tsxpackages/core/test/determine-coupon-to-apply.test.tspackages/core/src/schemas/stripe/checkout-session-metadata.tspackages/commerce-next/src/pricing/discount-badge.tsxpackages/core/test/format-prices-for-product.test.tspackages/core/test/pricing-test-fixtures.tspackages/core/src/lib/actions/prices-formatted.tspackages/core/test/get-calculated-price.test.tspackages/adapter-drizzle/src/lib/mysql/schemas/commerce/coupon.tspackages/core/src/providers/stripe.tsapps/ai-hero/src/app/admin/coupons/_components/coupon-generator-form.tsxpackages/core/src/lib/pricing/determine-coupon-to-apply.tspackages/core/src/lib/pricing/format-prices-for-product.tspackages/core/src/schemas/merchant-coupon-schema.tspackages/core/src/lib/pricing/get-calculated-price.tspackages/core/test/stripe-checkout-fixed-discounts.test.tsapps/ai-hero/src/lib/coupons-query.tspackages/adapter-drizzle/src/lib/mysql/schemas/commerce/merchant-coupon.tsapps/ai-hero/src/app/admin/coupons/_components/coupon-data-table.tsxpackages/core/src/lib/pricing/stripe-checkout.ts
packages/**/*
📄 CodeRabbit inference engine (.cursor/rules/monorepo-setup.mdc)
Packages are located in the /packages directory
Files:
packages/core/test/merchant-coupon-schema.test.tspackages/commerce-next/src/pricing/coupon-input.tsxpackages/commerce-next/src/pricing/price-display.tsxpackages/core/src/types.tspackages/commerce-next/src/pricing/pricing.tsxpackages/core/src/schemas/coupon-schema.tspackages/commerce-next/src/pricing/price-breakdown.tsxpackages/core/test/determine-coupon-to-apply.test.tspackages/core/src/schemas/stripe/checkout-session-metadata.tspackages/commerce-next/src/pricing/discount-badge.tsxpackages/core/test/format-prices-for-product.test.tspackages/core/test/pricing-test-fixtures.tspackages/core/src/lib/actions/prices-formatted.tspackages/core/test/get-calculated-price.test.tspackages/adapter-drizzle/src/lib/mysql/schemas/commerce/coupon.tspackages/core/src/providers/stripe.tspackages/core/src/lib/pricing/determine-coupon-to-apply.tspackages/core/src/lib/pricing/format-prices-for-product.tspackages/core/src/schemas/merchant-coupon-schema.tspackages/core/src/lib/pricing/get-calculated-price.tspackages/core/test/stripe-checkout-fixed-discounts.test.tspackages/adapter-drizzle/src/lib/mysql/schemas/commerce/merchant-coupon.tspackages/core/src/lib/pricing/stripe-checkout.ts
apps/**/*
📄 CodeRabbit inference engine (.cursor/rules/monorepo-setup.mdc)
Apps are located in the /apps directory
Files:
apps/ai-hero/src/app/admin/coupons/_components/coupon-generator-form.tsxapps/ai-hero/src/lib/coupons-query.tsapps/ai-hero/src/app/admin/coupons/_components/coupon-data-table.tsx
apps/ai-hero/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/ai-hero/.cursor/rules/analytics.mdc)
apps/ai-hero/**/*.{ts,tsx}: Thetrackfunction in@/utils/analytics.tsis ONLY for tracking learner/customer activity (such as course progress, video watching, exercise completion, purchase activity, and user preferences). Do NOT usetrackfor internal admin actions, content management, system operations, or backend processes.
Uselogfrom@/server/logger.tsfor system operations, content management, error tracking, performance monitoring, and backend processes.
Always include relevant IDs and context in log messages.Always use kebab-case when naming files
Add JSDoc comments to exported methods and React components in TypeScript files
Files:
apps/ai-hero/src/app/admin/coupons/_components/coupon-generator-form.tsxapps/ai-hero/src/lib/coupons-query.tsapps/ai-hero/src/app/admin/coupons/_components/coupon-data-table.tsx
apps/ai-hero/**/*.tsx
📄 CodeRabbit inference engine (apps/ai-hero/.cursor/rules/component-system.mdc)
Use base React components from the /packages/ui package when possible
Files:
apps/ai-hero/src/app/admin/coupons/_components/coupon-generator-form.tsxapps/ai-hero/src/app/admin/coupons/_components/coupon-data-table.tsx
🧬 Code graph analysis (11)
packages/core/test/merchant-coupon-schema.test.ts (1)
packages/core/src/schemas/merchant-coupon-schema.ts (1)
merchantCouponSchema(3-41)
packages/commerce-next/src/pricing/coupon-input.tsx (1)
packages/core/src/types.ts (1)
FormattedPrice(276-296)
packages/commerce-next/src/pricing/price-breakdown.tsx (1)
packages/core/src/types.ts (1)
FormattedPrice(276-296)
packages/core/test/determine-coupon-to-apply.test.ts (2)
packages/core/test/pricing-test-fixtures.ts (3)
createMockAdapter(194-312)testMerchantCoupons(85-152)testPurchases(157-178)packages/core/src/lib/pricing/determine-coupon-to-apply.ts (1)
determineCouponToApply(40-162)
packages/commerce-next/src/pricing/discount-badge.tsx (1)
packages/core/src/types.ts (1)
FormattedPrice(276-296)
packages/core/test/format-prices-for-product.test.ts (1)
packages/core/test/pricing-test-fixtures.ts (6)
createMockAdapter(194-312)testProducts(17-46)testPrices(51-80)testMerchantCoupons(85-152)testPurchases(157-178)testUpgradableProducts(183-189)
packages/core/test/pricing-test-fixtures.ts (5)
packages/core/src/schemas/product-schema.ts (1)
Product(46-46)packages/core/src/schemas/merchant-coupon-schema.ts (1)
MerchantCoupon(43-43)packages/core/src/schemas/purchase-schema.ts (1)
Purchase(39-39)packages/core/src/adapters.ts (1)
CourseBuilderAdapter(39-244)packages/core/src/types.ts (1)
PaymentsAdapter(131-196)
packages/core/test/get-calculated-price.test.ts (1)
packages/core/src/lib/pricing/get-calculated-price.ts (1)
getCalculatedPrice(18-46)
packages/core/src/lib/pricing/format-prices-for-product.ts (2)
packages/nodash/src/array.ts (1)
sum(77-78)packages/core/src/lib/pricing/determine-coupon-to-apply.ts (1)
determineCouponToApply(40-162)
packages/core/test/stripe-checkout-fixed-discounts.test.ts (3)
packages/core/src/types.ts (1)
PaymentsProviderConsumerConfig(121-129)packages/core/test/pricing-test-fixtures.ts (7)
createMockPaymentsAdapter(317-351)createMockAdapter(194-312)testMerchantCoupons(85-152)testProducts(17-46)testPrices(51-80)testPurchases(157-178)testUpgradableProducts(183-189)packages/core/src/lib/pricing/stripe-checkout.ts (1)
stripeCheckout(192-534)
apps/ai-hero/src/lib/coupons-query.ts (2)
apps/ai-hero/src/server/logger.ts (2)
log(16-51)error(36-38)packages/core/src/providers/stripe.ts (1)
createCoupon(140-143)
🪛 markdownlint-cli2 (0.18.1)
plans/flat-discount-coupons-decision-flow.md
128-128: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
134-134: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
168-168: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
182-182: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
201-201: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
220-220: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
233-233: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
250-250: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
259-259: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
269-269: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
plans/flat-discount-coupons-application-integration.md
877-877: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: e2e-node (course-builder-web)
- GitHub Check: typecheck
- GitHub Check: format
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (13)
packages/commerce-next/src/pricing/price-display.tsx (1)
23-24: LGTM! Well-implemented fixed discount support.The changes correctly:
- Extract
appliedFixedDiscountandappliedDiscountTypefromFormattedPrice- Use nullish coalescing (
?? 0) for safe percentage discount handling- Provide conditional rendering for fixed vs. percentage discounts
- Update accessibility labels appropriately for screen readers
- Maintain backward compatibility with existing percentage-based discount flows
The implementation is clean, type-safe, and accessible.
Also applies to: 29-29, 41-43, 66-72, 78-80
packages/core/src/providers/stripe.ts (1)
135-138: LGTM! Consistent implementation of getCouponAmountOff.The new method:
- Follows the existing
getCouponPercentOffpattern (lines 130-133)- Returns a safe default of
0whenamount_offis absent- Is properly mirrored in
mockStripeAdapterfor testing- Aligns with the
PaymentsAdapterinterface extension mentioned in the planning documentsThe implementation is straightforward and consistent with the codebase patterns.
Also applies to: 240-240
apps/ai-hero/src/lib/coupons-query.ts (1)
153-153: Design decision verified: null identifiers handled correctly
stripeCheckout’s amountDiscount branch creates transient coupons for fixed discounts, coupons-query.ts documents “No Stripe coupon ID for fixed discounts,” and adapter mapping logic inserts a new merchantCoupon using the generated stripeCouponId when identifier is null. No changes needed.packages/adapter-drizzle/src/lib/mysql/schemas/commerce/merchant-coupon.ts (1)
22-26: Add missing migration and enforce check constraint
No migration was found underpackages/adapter-drizzle/...for addingamountDiscount; please add a migration that (1) adds the nullableamountDiscountcolumn and (2) defines a DB check constraint enforcing that only one ofpercentageDiscountoramountDiscountis non-null.packages/core/src/lib/pricing/stripe-checkout.ts (2)
306-311: LGTM: Consistent pattern with existing percentage discount retrieval.The
stripeCouponAmountOffcalculation follows the same defensive pattern asstripeCouponPercentOff, using the payments adapter and returning 0 as a safe default.
359-359: availableUpgrade indexing is correct
adapter.getUpgradableProductsreturnsPromise<UpgradableProduct[]>, soavailableUpgradeisUpgradableProduct[] | nullandavailableUpgrade?.[0] || nullproperly yields the first element ornull.Likely an incorrect or invalid review comment.
packages/commerce-next/src/pricing/coupon-input.tsx (2)
42-55: LGTM: Clean error handling and loading state management.The apply handler properly validates input, manages loading state, and catches errors with user-friendly feedback.
67-84: Fixed discount already in dollars—no UI conversion needed
appliedFixedDiscountis set toamountDiscount / 100informatPricesForProduct, so calling.toFixed(2)correctly displays the dollar value. No change required.packages/core/src/lib/pricing/get-calculated-price.ts (1)
27-40: LGTM: Clear separation of discount types prevents double-discounting.The calculation correctly:
- Applies upgrade discount first (fixed discount for upgrades)
- Then applies exactly one merchant coupon discount (fixed XOR percentage)
- Converts cents to dollars for fixed amounts (line 34)
The mutually exclusive paths at lines 32-40 prevent revenue leaks from double-discounting.
plans/flat-discount-coupons-architecture.md (1)
1-552: Well-structured architecture documentation.This comprehensive planning document provides valuable context for understanding the PR changes, including:
- System architecture diagrams
- Data flow visualizations
- Implementation phases
- Risk mitigation strategies
- Success criteria
Minor note: Verify that the actual implementation aligns with this plan, especially the discount precedence rules and calculation order documented in sections 2-3.
packages/commerce-next/src/pricing/discount-badge.tsx (2)
50-63: Fixed discount rounded to whole dollars - verify if intentional.Line 53 formats the discount amount with
toFixed(0), which truncates cents (e.g., $20.50 displays as "Save $20"). This may be intentional for cleaner badge display.If you want to preserve cents precision, update the formatting:
case 'fixed': - return `Save $${discountAmount.toFixed(0)}` + return `Save $${discountAmount.toFixed(2)}`If whole dollar display is preferred for badges, consider adding a JSDoc comment explaining this choice.
39-48: LGTM: Clean component structure with appropriate early return.The component correctly returns null when no discount exists and provides sensible size options for different use cases.
packages/core/test/determine-coupon-to-apply.test.ts (1)
1-495: Excellent comprehensive test coverage.The test suite thoroughly exercises
determineCouponToApplywith:
- Fixed vs percentage discount detection
- Priority logic (PPP > Bulk > Fixed > Percentage)
- Edge cases (zero amounts, null values, undefined IDs)
- Coupon restrictions
- Return value structure validation
- Mock adapter pattern for isolation
The tests align well with the implementation changes and provide confidence in the discount selection logic. The use of shared test fixtures (from
pricing-test-fixtures.ts) promotes consistency across test files.
apps/ai-hero/src/app/admin/coupons/_components/coupon-generator-form.tsx
Show resolved
Hide resolved
| percentageDiscount: decimal('percentageDiscount', { | ||
| precision: 3, | ||
| scale: 2, | ||
| }).notNull(), | ||
| }), | ||
| amountDiscount: int('amountDiscount'), |
There was a problem hiding this comment.
🧩 Analysis chain
Ensure database migration for amountDiscount is complete.
Similar to the MerchantCoupon schema, the Coupon table now includes an amountDiscount field. Verify that:
- The database migration has been applied.
- Existing rows handle the new nullable column appropriately.
- Application code that queries coupons accounts for the new field.
Run the following script to check for migration files:
🏁 Script executed:
#!/bin/bash
# Search for migration files affecting the Coupon table
fd -e sql . packages/adapter-drizzle/drizzle/migrations/ | xargs grep -l "Coupon.*amountDiscount"Length of output: 131
🏁 Script executed:
#!/bin/bash
# Find all migrations directories in the repository
echo "Migrations directories found:"
fd -t d migrations | head -n 20
# Search for 'amountDiscount' in migration files within those directories
echo "Searching for 'amountDiscount' in migration files:"
for dir in $(fd -t d migrations); do
echo "Directory: $dir"
fd -t f -e sql -e ts . "$dir" | xargs grep -n "amountDiscount" || echo "No matches in $dir"
doneLength of output: 80
Add migration for the new amountDiscount column on Coupon: create and apply a database migration for this field and handle existing rows appropriately.
🤖 Prompt for AI Agents
In packages/adapter-drizzle/src/lib/mysql/schemas/commerce/coupon.ts around
lines 36–40 you added amountDiscount to the schema but did not create a DB
migration; create a migration that adds an integer column amountDiscount to the
coupons table, make it nullable initially, backfill existing rows (e.g., set to
0 or compute a value if business logic requires), and then optionally alter the
column to NOT NULL with a default if the application expects it non-null; apply
the migration, ensure the ORM schema matches the final column definition, and
include tests or a rollback step to validate the change.
| const hasDiscount = calculatedPrice < fullPrice | ||
| const discountAmount = fullPrice - calculatedPrice | ||
|
|
||
| return ( | ||
| <div className={cn('space-y-2', className)}> | ||
| {/* Original Price */} | ||
| {hasDiscount && ( | ||
| <div className="text-muted-foreground flex justify-between text-sm"> | ||
| <span>Original Price</span> | ||
| <span className="line-through">${fullPrice.toFixed(2)}</span> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Discount Breakdown */} | ||
| {showDetails && hasDiscount && ( | ||
| <div className="space-y-1"> | ||
| {/* Fixed Discount */} | ||
| {appliedDiscountType === 'fixed' && appliedFixedDiscount && ( | ||
| <div className="flex justify-between text-sm text-green-600 dark:text-green-400"> | ||
| <span>Discount ({appliedMerchantCoupon?.type})</span> | ||
| <span>-${appliedFixedDiscount.toFixed(2)}</span> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Percentage Discount */} | ||
| {['percentage', 'ppp', 'bulk'].includes( | ||
| appliedDiscountType || '', | ||
| ) && ( | ||
| <div className="flex justify-between text-sm text-green-600 dark:text-green-400"> | ||
| <span> | ||
| Discount ( | ||
| {appliedMerchantCoupon?.percentageDiscount | ||
| ? `${(appliedMerchantCoupon.percentageDiscount * 100).toFixed(0)}%` | ||
| : appliedDiscountType} | ||
| ) | ||
| </span> | ||
| <span>-${discountAmount.toFixed(2)}</span> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Potential discrepancy in discount amount display.
Lines 38 and 73 both calculate discountAmount from fullPrice - calculatedPrice. However, when both an upgrade credit (fixedDiscountForUpgrade) and a merchant discount exist, this calculation includes both discounts, which may not match the merchant coupon discount alone.
For percentage discounts (lines 62-75), consider displaying the merchant discount separately:
{['percentage', 'ppp', 'bulk'].includes(
appliedDiscountType || '',
) && (
<div className="flex justify-between text-sm text-green-600 dark:text-green-400">
<span>
Discount (
{appliedMerchantCoupon?.percentageDiscount
? `${(appliedMerchantCoupon.percentageDiscount * 100).toFixed(0)}%`
: appliedDiscountType}
)
</span>
- <span>-${discountAmount.toFixed(2)}</span>
+ <span>
+ -${(fullPrice * (appliedMerchantCoupon?.percentageDiscount || 0)).toFixed(2)}
+ </span>
</div>
)}This ensures the displayed amount matches the merchant coupon discount, excluding upgrade credits.
📝 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 hasDiscount = calculatedPrice < fullPrice | |
| const discountAmount = fullPrice - calculatedPrice | |
| return ( | |
| <div className={cn('space-y-2', className)}> | |
| {/* Original Price */} | |
| {hasDiscount && ( | |
| <div className="text-muted-foreground flex justify-between text-sm"> | |
| <span>Original Price</span> | |
| <span className="line-through">${fullPrice.toFixed(2)}</span> | |
| </div> | |
| )} | |
| {/* Discount Breakdown */} | |
| {showDetails && hasDiscount && ( | |
| <div className="space-y-1"> | |
| {/* Fixed Discount */} | |
| {appliedDiscountType === 'fixed' && appliedFixedDiscount && ( | |
| <div className="flex justify-between text-sm text-green-600 dark:text-green-400"> | |
| <span>Discount ({appliedMerchantCoupon?.type})</span> | |
| <span>-${appliedFixedDiscount.toFixed(2)}</span> | |
| </div> | |
| )} | |
| {/* Percentage Discount */} | |
| {['percentage', 'ppp', 'bulk'].includes( | |
| appliedDiscountType || '', | |
| ) && ( | |
| <div className="flex justify-between text-sm text-green-600 dark:text-green-400"> | |
| <span> | |
| Discount ( | |
| {appliedMerchantCoupon?.percentageDiscount | |
| ? `${(appliedMerchantCoupon.percentageDiscount * 100).toFixed(0)}%` | |
| : appliedDiscountType} | |
| ) | |
| </span> | |
| <span>-${discountAmount.toFixed(2)}</span> | |
| </div> | |
| )} | |
| {['percentage', 'ppp', 'bulk'].includes( | |
| appliedDiscountType || '', | |
| ) && ( | |
| <div className="flex justify-between text-sm text-green-600 dark:text-green-400"> | |
| <span> | |
| Discount ( | |
| {appliedMerchantCoupon?.percentageDiscount | |
| ? `${(appliedMerchantCoupon.percentageDiscount * 100).toFixed(0)}%` | |
| : appliedDiscountType} | |
| ) | |
| </span> | |
| <span> | |
| -${(fullPrice * (appliedMerchantCoupon?.percentageDiscount || 0)).toFixed(2)} | |
| </span> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In packages/commerce-next/src/pricing/price-breakdown.tsx around lines 37 to 75,
the displayed discount amount uses discountAmount = fullPrice - calculatedPrice
which mixes upgrade credit (fixedDiscountForUpgrade) with merchant coupon
discounts; when both exist the merchant coupon line shows the combined value
instead of the merchant-only discount. Fix by calculating a separate
merchantDiscountAmount that excludes any upgrade credit (e.g.,
merchantDiscountAmount = discountAmount - (fixedDiscountForUpgrade ?? 0) or, for
fixed merchant coupons, use appliedFixedDiscount directly), use
appliedFixedDiscount for the fixed-discount UI, and use merchantDiscountAmount
for the percentage/ppp/bulk UI; ensure you guard against undefined values and
format toFixed(2) when rendering.
| appliedFixedDiscount ? ( | ||
| <div className="text-primary text-base"> | ||
| Save ${appliedFixedDiscount.toFixed(0)} | ||
| </div> | ||
| ) : ( | ||
| <div className="text-primary text-base"> | ||
| Save {percentOff}% | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Fixed discount label rounds away real value
toFixed(0) rounds ${appliedFixedDiscount} to a whole dollar, so a $25.50 fixed coupon would render “Save $26”. That misstates the customer savings. Please keep cents (e.g. toFixed(2) or reuse formatUsd) so the displayed discount matches the actual value.
🤖 Prompt for AI Agents
In packages/commerce-next/src/pricing/pricing.tsx around lines 280 to 288, the
fixed discount display is using appliedFixedDiscount.toFixed(0) which rounds
cents away (e.g. $25.50 -> $26); change the rendering to preserve cents by
formatting the value to two decimal places or reusing the app's currency
formatter (e.g. use toFixed(2) or call formatUsd(appliedFixedDiscount)) so the
displayed "Save $X.XX" matches the actual discount value.
| if (merchantCoupon.amountDiscount) { | ||
| // For fixed amount discounts, create a transient coupon | ||
| const couponId = await config.paymentsAdapter.createCoupon({ | ||
| amount_off: merchantCoupon.amountDiscount, | ||
| name: merchantCoupon.type || 'Fixed Discount', | ||
| max_redemptions: 1, | ||
| expires_at: TWELVE_FOUR_HOURS_FROM_NOW, | ||
| redeem_by: TWELVE_FOUR_HOURS_FROM_NOW, | ||
| currency: 'USD', | ||
| applies_to: { | ||
| products: [merchantProductIdentifier], | ||
| }, | ||
| }) | ||
| discounts.push({ | ||
| promotion_code: promotionCodeId, | ||
| }) | ||
|
|
||
| discounts.push({ | ||
| coupon: couponId, | ||
| }) |
There was a problem hiding this comment.
Currency hardcoded to USD - consider internationalization.
The fixed amount discount path creates Stripe coupons with currency: 'USD' hardcoded (line 398). This may cause issues for merchants operating in other currencies.
Consider:
- Retrieving the currency from the product/price
- Storing currency in merchant settings
- Adding a currency field to the merchant coupon schema
If USD-only is intentional, document this limitation in the code comments.
🤖 Prompt for AI Agents
In packages/core/src/lib/pricing/stripe-checkout.ts around lines 391 to 406, the
coupon creation hardcodes currency: 'USD' which breaks non-USD merchants; update
the code to determine currency dynamically (preferred) by reading the
product/price currency or merchant settings (e.g., merchant.currency ||
price.currency) and pass that value to createCoupon, validate it against
supported Stripe currencies, and fall back to a safe default only if necessary;
if the project requires persisting coupon currency, add a currency field to the
merchant coupon schema and populate it, or if USD-only is intentional, replace
the literal with a clear inline comment describing the limitation and reason.
| ...(merchantCoupon && { | ||
| discountType: merchantCoupon.amountDiscount ? 'fixed' : 'percentage', | ||
| discountAmount: merchantCoupon.amountDiscount | ||
| ? merchantCoupon.amountDiscount | ||
| : stripeCouponPercentOff * 100, | ||
| }), |
There was a problem hiding this comment.
Inconsistent units in discountAmount metadata field.
The discountAmount field stores different units depending on discountType:
fixed: stores amount in cents (e.g., 2000 for $20)percentage: stores percentage points (e.g., 25 for 25%)
This inconsistency could cause confusion when processing metadata downstream (webhooks, analytics, etc.).
Consider using separate fields:
...(merchantCoupon && {
discountType: merchantCoupon.amountDiscount ? 'fixed' : 'percentage',
- discountAmount: merchantCoupon.amountDiscount
- ? merchantCoupon.amountDiscount
- : stripeCouponPercentOff * 100,
+ ...(merchantCoupon.amountDiscount && {
+ discountAmountCents: merchantCoupon.amountDiscount,
+ }),
+ ...(stripeCouponPercentOff > 0 && {
+ discountPercentage: stripeCouponPercentOff * 100,
+ }),
}),Or document the unit behavior clearly in the metadata schema.
📝 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.
| ...(merchantCoupon && { | |
| discountType: merchantCoupon.amountDiscount ? 'fixed' : 'percentage', | |
| discountAmount: merchantCoupon.amountDiscount | |
| ? merchantCoupon.amountDiscount | |
| : stripeCouponPercentOff * 100, | |
| }), | |
| ...(merchantCoupon && { | |
| discountType: merchantCoupon.amountDiscount ? 'fixed' : 'percentage', | |
| - discountAmount: merchantCoupon.amountDiscount | |
| - ? merchantCoupon.amountDiscount | |
| ...(merchantCoupon.amountDiscount && { | |
| discountAmountCents: merchantCoupon.amountDiscount, | |
| }), | |
| ...(stripeCouponPercentOff > 0 && { | |
| discountPercentage: stripeCouponPercentOff * 100, | |
| }), | |
| }), |
🤖 Prompt for AI Agents
In packages/core/src/lib/pricing/stripe-checkout.ts around lines 467 to 472, the
metadata field discountAmount mixes units (cents for fixed discounts, percentage
points for percentage discounts); change the metadata to use explicit,
consistent fields instead: when merchantCoupon.amountDiscount is present set
discountAmountCents to the cent value and set discountType='fixed', otherwise
set discountPercent (as an integer or decimal percent) and
discountType='percentage'; ensure you remove or stop writing the ambiguous
discountAmount field and update any callers/consumers (webhooks, analytics) to
read the new fields.
| discountAmount: z.number().optional(), | ||
| }) |
There was a problem hiding this comment.
Coerce Stripe metadata values to numbers before validation
Stripe metadata values are returned as strings. Parsing them with z.number() will throw when webhooks read the session metadata. Please switch to z.coerce.number().optional() (or preprocess the value) so string inputs like '20' validate correctly.
🤖 Prompt for AI Agents
In packages/core/src/schemas/stripe/checkout-session-metadata.ts around lines 17
to 18, the schema uses z.number().optional() which will reject Stripe metadata
(returned as strings); change that to z.coerce.number().optional() (or add a
preprocess step that parses numeric strings to numbers) so values like '20'
validate correctly, and update any related imports/types if needed.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/code-with-antonio/src/trpc/api/routers/pricing.ts (1)
203-206: Add null-safety for percentageDiscount access.Line 205 calls
.toString()ondefaultCoupon.percentageDiscountwithout verifying it exists. If the PR introduces fixed amount coupons (which wouldn't havepercentageDiscount), this will throw a runtime error.Consider adding a null-check or type guard:
const minimalDefaultCoupon = defaultCoupon && { expires: defaultCoupon.expires?.toISOString(), - percentageDiscount: defaultCoupon.percentageDiscount.toString(), + percentageDiscount: defaultCoupon.percentageDiscount?.toString(), }Alternatively, if the response schema requires this field, ensure
defaultCouponis filtered to only percentage-based coupons before reaching this point.apps/course-builder-web/src/trpc/api/routers/pricing.ts (1)
109-117: Add JSDoc comment.Per coding guidelines, functions should have JSDoc comments documenting parameters, return values, and behavior.
As per coding guidelines.
+/** + * Determines which merchant coupon to apply by comparing incoming and default coupons. + * @param productId - The product ID to check for default coupons + * @param siteCouponId - Optional coupon ID from user input + * @param code - Optional coupon code from user input + * @returns Object containing the selected merchant coupon and related metadata + */ async function getActiveMerchantCoupon({ productId, siteCouponId, code, }: { productId: string | undefined siteCouponId: string | undefined code: string | undefined }) {
♻️ Duplicate comments (1)
apps/ai-hero/src/lib/sale-banner.ts (1)
54-72: Past review concern: Mutual exclusivity still not enforced.The previous review flagged that the coupon schema allows both
amountDiscountandpercentageDiscountto be set simultaneously. This concern remains unaddressed—the code prioritizes fixed discounts when both are present (line 55), but schema-level validation would prevent ambiguity at the source.As per the past review comment, consider adding a Zod refinement in the coupon schema to enforce that exactly one discount type is provided. This would prevent invalid states and make the prioritization logic here unnecessary.
Based on past review comments.
🧹 Nitpick comments (2)
apps/code-with-antonio/src/trpc/api/routers/pricing.ts (1)
1-16: Reorder imports per coding guidelines.The import order doesn't follow the specified pattern (React → Next → 3rd party → internal). Third-party imports (
date-fns,zod) should appear before internal imports.Apply this diff to reorder the imports:
import { headers } from 'next/headers' -import { courseBuilderAdapter, db } from '@/db' -import { getServerAuthSession } from '@/server/auth' -import { createTRPCRouter, publicProcedure } from '@/trpc/api/trpc' import { isAfter } from 'date-fns' import { z } from 'zod' +import { courseBuilderAdapter, db } from '@/db' +import { getServerAuthSession } from '@/server/auth' +import { createTRPCRouter, publicProcedure } from '@/trpc/api/trpc' import { formatPricesForProduct } from '@coursebuilder/core' import { propsForCommerce } from '@coursebuilder/core/pricing/props-for-commerce'As per coding guidelines.
apps/ai-hero/src/utils/discount-formatter.ts (1)
22-27: Redundant null check and unnecessary type guard.Line 24 checks
coupon.amountDiscountagain afterhasFixedDiscountalready verifies it's truthy and > 0. TypeScript's control flow analysis should narrow the type without the redundant check in the condition.Apply this diff to simplify:
- const hasFixedDiscount = coupon.amountDiscount && coupon.amountDiscount > 0 - - if (hasFixedDiscount && coupon.amountDiscount) { + if (coupon.amountDiscount && coupon.amountDiscount > 0) { // Fixed amount discount (in cents, convert to dollars) const discountValue = coupon.amountDiscount / 100 return `$${discountValue.toFixed(2)}`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/ai-hero/src/hooks/use-sale-toast-notifier.tsx(2 hunks)apps/ai-hero/src/lib/sale-banner.ts(2 hunks)apps/ai-hero/src/utils/discount-formatter.ts(1 hunks)apps/code-with-antonio/src/trpc/api/routers/pricing.ts(1 hunks)apps/course-builder-web/src/trpc/api/routers/pricing.ts(1 hunks)apps/epicdev-ai/src/trpc/api/routers/pricing.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/ai-hero/src/hooks/use-sale-toast-notifier.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Add JS Doc comments to functions and React components
Files:
apps/ai-hero/src/lib/sale-banner.tsapps/code-with-antonio/src/trpc/api/routers/pricing.tsapps/epicdev-ai/src/trpc/api/routers/pricing.tsapps/course-builder-web/src/trpc/api/routers/pricing.tsapps/ai-hero/src/utils/discount-formatter.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use single quotes, no semicolons, tabs (width: 2), and an 80 character line limit for code formatting.
Organize imports in the following order: React → Next → 3rd party → internal.
Do NOT use Object.defineProperty(exports, ...) for re-exports. Use standard export patterns instead to avoid conflicts with framework internals.
Files:
apps/ai-hero/src/lib/sale-banner.tsapps/code-with-antonio/src/trpc/api/routers/pricing.tsapps/epicdev-ai/src/trpc/api/routers/pricing.tsapps/course-builder-web/src/trpc/api/routers/pricing.tsapps/ai-hero/src/utils/discount-formatter.ts
apps/**/*
📄 CodeRabbit inference engine (.cursor/rules/monorepo-setup.mdc)
Apps are located in the /apps directory
Files:
apps/ai-hero/src/lib/sale-banner.tsapps/code-with-antonio/src/trpc/api/routers/pricing.tsapps/epicdev-ai/src/trpc/api/routers/pricing.tsapps/course-builder-web/src/trpc/api/routers/pricing.tsapps/ai-hero/src/utils/discount-formatter.ts
apps/ai-hero/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/ai-hero/.cursor/rules/analytics.mdc)
apps/ai-hero/**/*.{ts,tsx}: Thetrackfunction in@/utils/analytics.tsis ONLY for tracking learner/customer activity (such as course progress, video watching, exercise completion, purchase activity, and user preferences). Do NOT usetrackfor internal admin actions, content management, system operations, or backend processes.
Uselogfrom@/server/logger.tsfor system operations, content management, error tracking, performance monitoring, and backend processes.
Always include relevant IDs and context in log messages.Always use kebab-case when naming files
Add JSDoc comments to exported methods and React components in TypeScript files
Files:
apps/ai-hero/src/lib/sale-banner.tsapps/ai-hero/src/utils/discount-formatter.ts
apps/code-with-antonio/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/code-with-antonio/.cursor/rules/analytics.mdc)
apps/code-with-antonio/**/*.{ts,tsx}: Only usetrackfrom@/utils/analytics.tsfor learner/customer activity (course progress, video watching, exercise completion, purchase activity, user preferences)
Do not usetrackfor internal admin actions, content management, system operations, or backend processesAlways use kebab-case when naming TypeScript and TSX files
apps/code-with-antonio/**/*.{ts,tsx}: Add JSDoc comments to exported methods/functions in TypeScript
Add JSDoc comments to exported React components
Files:
apps/code-with-antonio/src/trpc/api/routers/pricing.ts
apps/epicdev-ai/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/epicdev-ai/.cursor/rules/analytics.mdc)
apps/epicdev-ai/**/*.{ts,tsx}: Thetrackfunction in@/utils/analytics.tsis ONLY for tracking learner/customer activity (such as course progress, video watching, exercise completion, purchase activity, and user preferences). DO NOT usetrackfor internal admin actions, content management, system operations, or backend processes.
Uselogfrom@/server/logger.tsfor system operations, content management, error tracking, performance monitoring, and backend processes.
Always include relevant IDs and context in log messages.Always use kebab-case when naming files
Add JSDoc comments to exported methods and React components in TypeScript files
Files:
apps/epicdev-ai/src/trpc/api/routers/pricing.ts
apps/*/src/utils/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
When creating shared utility packages, use the re-export pattern to maintain backward compatibility. For example, re-export from the shared package in app-specific utility files.
Files:
apps/ai-hero/src/utils/discount-formatter.ts
🧬 Code graph analysis (1)
apps/ai-hero/src/lib/sale-banner.ts (1)
apps/ai-hero/src/utils/discount-formatter.ts (1)
formatDiscount(18-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build
- GitHub Check: typecheck
- GitHub Check: lint
- GitHub Check: format
- GitHub Check: e2e-node (course-builder-web)
🔇 Additional comments (2)
apps/ai-hero/src/lib/sale-banner.ts (2)
9-17: LGTM! Well-structured type definition with backward compatibility.The new
SaleBannerDatatype clearly distinguishes discount types while preserving the optionalpercentOfffield for backward compatibility. The field naming is consistent and self-documenting.
72-72: Approve use of formatDiscount utility for consistent UI display.Calling
formatDiscount(coupon)ensures discount formatting is centralized and consistent across the UI.
| // Determine discount type and format | ||
| const hasFixedDiscount = coupon.amountDiscount && coupon.amountDiscount > 0 | ||
| const discountType = hasFixedDiscount ? 'fixed' : 'percentage' | ||
|
|
||
| let discountValue: number | ||
| let percentOff: number | undefined | ||
|
|
||
| if (hasFixedDiscount && coupon.amountDiscount) { | ||
| // Fixed amount discount (in cents, convert to dollars) | ||
| discountValue = coupon.amountDiscount / 100 | ||
| } else { | ||
| // Percentage discount | ||
| percentOff = parseFloat( | ||
| (Number(coupon.percentageDiscount) * 100).toFixed(1), | ||
| ) | ||
| discountValue = percentOff | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Eliminate logic duplication by relying on the formatDiscount utility.
Lines 54-70 duplicate the discount type determination logic already present in formatDiscount. This violates DRY and creates a maintenance burden—changes to discount logic must be synchronized across both locations.
Additionally, line 61 has the same redundant null check as the formatter, and lines 66-69 have the same NaN risk when percentageDiscount is invalid.
Consider refactoring to extract the discount type determination into a shared utility or leverage formatDiscount more directly:
- // Determine discount type and format
- const hasFixedDiscount = coupon.amountDiscount && coupon.amountDiscount > 0
- const discountType = hasFixedDiscount ? 'fixed' : 'percentage'
-
- let discountValue: number
- let percentOff: number | undefined
-
- if (hasFixedDiscount && coupon.amountDiscount) {
- // Fixed amount discount (in cents, convert to dollars)
- discountValue = coupon.amountDiscount / 100
- } else {
- // Percentage discount
- percentOff = parseFloat(
- (Number(coupon.percentageDiscount) * 100).toFixed(1),
- )
- discountValue = percentOff
- }
+ // Determine discount type and value
+ const discountType = (coupon.amountDiscount && coupon.amountDiscount > 0) ? 'fixed' : 'percentage'
+
+ let discountValue: number
+ let percentOff: number | undefined
+
+ if (discountType === 'fixed' && coupon.amountDiscount) {
+ discountValue = coupon.amountDiscount / 100
+ } else {
+ const percentNum = Number(coupon.percentageDiscount) * 100
+ percentOff = isNaN(percentNum) ? 0 : parseFloat(percentNum.toFixed(1))
+ discountValue = percentOff
+ }
const discountFormatted = formatDiscount(coupon)Or, extract a shared getDiscountType utility that both functions can use.
📝 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.
| // Determine discount type and format | |
| const hasFixedDiscount = coupon.amountDiscount && coupon.amountDiscount > 0 | |
| const discountType = hasFixedDiscount ? 'fixed' : 'percentage' | |
| let discountValue: number | |
| let percentOff: number | undefined | |
| if (hasFixedDiscount && coupon.amountDiscount) { | |
| // Fixed amount discount (in cents, convert to dollars) | |
| discountValue = coupon.amountDiscount / 100 | |
| } else { | |
| // Percentage discount | |
| percentOff = parseFloat( | |
| (Number(coupon.percentageDiscount) * 100).toFixed(1), | |
| ) | |
| discountValue = percentOff | |
| } | |
| // Determine discount type and value | |
| const discountType = (coupon.amountDiscount && coupon.amountDiscount > 0) ? 'fixed' : 'percentage' | |
| let discountValue: number | |
| let percentOff: number | undefined | |
| if (discountType === 'fixed' && coupon.amountDiscount) { | |
| discountValue = coupon.amountDiscount / 100 | |
| } else { | |
| const percentNum = Number(coupon.percentageDiscount) * 100 | |
| percentOff = isNaN(percentNum) ? 0 : parseFloat(percentNum.toFixed(1)) | |
| discountValue = percentOff | |
| } | |
| const discountFormatted = formatDiscount(coupon) |
| } else { | ||
| // Percentage discount | ||
| const percentOff = Number(coupon.percentageDiscount) * 100 | ||
| return `${percentOff}%` | ||
| } |
There was a problem hiding this comment.
Handle invalid percentageDiscount values to prevent "NaN%" output.
If percentageDiscount is null, undefined, or an invalid string, Number(coupon.percentageDiscount) returns NaN, resulting in "NaN%" being returned. This creates a user-facing display bug.
Apply this diff to add validation:
} else {
// Percentage discount
const percentOff = Number(coupon.percentageDiscount) * 100
- return `${percentOff}%`
+ if (isNaN(percentOff)) {
+ return '0%'
+ }
+ return `${percentOff}%`
}Alternatively, validate at the call site or return a default like '0%' for invalid inputs.
📝 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.
| } else { | |
| // Percentage discount | |
| const percentOff = Number(coupon.percentageDiscount) * 100 | |
| return `${percentOff}%` | |
| } | |
| } else { | |
| // Percentage discount | |
| const percentOff = Number(coupon.percentageDiscount) * 100 | |
| if (isNaN(percentOff)) { | |
| return '0%' | |
| } | |
| return `${percentOff}%` | |
| } |
🤖 Prompt for AI Agents
In apps/ai-hero/src/utils/discount-formatter.ts around lines 28 to 32, the
percentage-branch converts coupon.percentageDiscount using Number(...) and may
return "NaN%" for null/undefined/invalid inputs; validate the value before
formatting by checking it's a finite number (e.g., use Number(...) and
Number.isFinite or parseFloat + isFinite) and if invalid return a safe default
like "0%" or an empty string; update the return to use the validated numeric
value times 100 and format (including trimming decimals if needed).
| // this application only supports percentage discounts | ||
| // so we only need to check the percentage discount | ||
| const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon | ||
| if ( | ||
| incomingMerchantCoupon?.percentageDiscount && | ||
| incomingMerchantCoupon.percentageDiscount >= | ||
| defaultMerchantCoupon.percentageDiscount | ||
| defaultMerchantCoupon.percentageDiscount | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
Logic doesn't support fixed amount discounts despite PR objectives.
The PR title and description indicate adding fixed amount discount support (e.g., "$20 off", "$100 off") with comparison logic to choose the better discount. However, this code path contradicts that goal:
- Comments at lines 140-141 explicitly state "this application only supports percentage discounts"
- The comparison logic only checks
percentageDiscountvalues - If
incomingMerchantCouponis a fixed amount coupon (nopercentageDiscount), the null-check at line 144 fails, and the code falls through to usedefaultMerchantCouponinstead - The AI summary claims discounts are "compared by monetary value using unitPrice", but this code only compares percentage numbers
This means fixed amount coupons cannot be properly selected or compared in this code path, completely bypassing the feature described in the PR.
To properly support fixed amount discounts, you need to:
- Compare monetary values (percentage of price vs. fixed amount) rather than just percentage numbers
- Handle cases where
incomingMerchantCouponhasamountDiscountinstead ofpercentageDiscount - Update the comments to reflect the new capability
Run the following script to verify if fixed amount discount logic exists elsewhere in the PR:
🏁 Script executed:
#!/bin/bash
# Search for amountDiscount or discountType handling in the codebase
echo "=== Searching for amountDiscount field usage ==="
rg -n 'amountDiscount' --type ts --type tsx -C 3
echo -e "\n=== Searching for discountType field usage ==="
rg -n 'discountType' --type ts --type tsx -C 3
echo -e "\n=== Searching for fixed discount comparison logic ==="
rg -nP '(comparing|compare|monetary|amount.*discount|fixed.*discount)' --type ts --type tsx -C 2Length of output: 235
🏁 Script executed:
#!/bin/bash
echo "=== amountDiscount occurrences ==="
rg -n 'amountDiscount' -g '*.ts' -g '*.tsx' -C3
echo "=== discountType occurrences ==="
rg -n 'discountType' -g '*.ts' -g '*.tsx' -C3
echo "=== pricing.ts context around incomingMerchantCoupon ==="
rg -n 'incomingMerchantCoupon' -C5 apps/code-with-antonio/src/trpc/api/routers/pricing.tsLength of output: 87776
Compare fixed and percentage discounts in pricing router
In apps/code-with-antonio/src/trpc/api/routers/pricing.ts (lines 140–147), the logic only checks percentageDiscount, so coupons with amountDiscount never win. Calculate both discount amounts (e.g. amountDiscount/100*quantity vs percentageDiscount*unitPrice), choose the larger, and update the comments.
🤖 Prompt for AI Agents
In apps/code-with-antonio/src/trpc/api/routers/pricing.ts around lines 140 to
147, the current check only compares percentageDiscounts so coupons that use
amountDiscount never win; compute the actual discount value for both
incomingMerchantCoupon and defaultMerchantCoupon by calculating
amountDiscountValue = (amountDiscount / 100) * quantity (or multiply by
cents/units as appropriate) and percentageDiscountValue = (percentageDiscount /
100) * unitPrice * quantity, choose the larger of the two for each coupon
(handling absent values as zero), then compare the two computed totals and
update the if-condition and comments to use that computed greater-discount
comparison so amount-based coupons are correctly considered.
| // this application only supports percentage discounts | ||
| // so we only need to check the percentage discount | ||
| const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon | ||
| if ( | ||
| incomingMerchantCoupon?.percentageDiscount && | ||
| incomingMerchantCoupon.percentageDiscount >= | ||
| defaultMerchantCoupon.percentageDiscount | ||
| defaultMerchantCoupon.percentageDiscount |
There was a problem hiding this comment.
Fixed-amount coupons cannot win over default percentage coupons.
The null-safe guard on line 145 causes fixed-amount coupons (which lack a percentageDiscount property) to always lose to percentage-based default coupons. When incomingMerchantCoupon.percentageDiscount is undefined/null, the condition fails and execution falls through to line 152, selecting defaultMerchantCoupon instead.
This contradicts the PR's objective. According to the screenshots, a $100 fixed discount should win over a 40% discount when it provides greater savings. However, this code cannot perform that comparison because:
- It only checks
percentageDiscount, ignoringamountDiscount. - The function lacks access to
unitPriceneeded to convert percentages and fixed amounts into comparable monetary values.
Additionally, the comments on lines 141-142 claim "this application only supports percentage discounts," which is now outdated given that this PR adds fixed-amount discount support.
To properly compare fixed vs. percentage discounts, this function needs:
async function getActiveMerchantCoupon({
productId,
siteCouponId,
code,
+ unitPrice,
}: {
productId: string | undefined
siteCouponId: string | undefined
code: string | undefined
+ unitPrice: number
}) {Then update the comparison logic to calculate actual discount amounts:
if (
// compare the discounts if there is a coupon and site/sale running
incomingCoupon?.merchantCoupon &&
couponIsValid(incomingCoupon) &&
defaultMerchantCoupon
) {
- // use whichever coupon provides the bigger discount
- // this application only supports percentage discounts
- // so we only need to check the percentage discount
+ // use whichever coupon provides the bigger discount in monetary terms
const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon
+ const incomingDiscount = incomingMerchantCoupon.amountDiscount
+ ?? (incomingMerchantCoupon.percentageDiscount * unitPrice / 100)
+ const defaultDiscount = defaultMerchantCoupon.amountDiscount
+ ?? (defaultMerchantCoupon.percentageDiscount * unitPrice / 100)
+
- if (
- incomingMerchantCoupon?.percentageDiscount &&
- incomingMerchantCoupon.percentageDiscount >=
- defaultMerchantCoupon.percentageDiscount
- ) {
+ if (incomingDiscount >= defaultDiscount) {
activeMerchantCoupon = incomingMerchantCoupon
usedCouponId = incomingCoupon.id
} else {
activeMerchantCoupon = defaultMerchantCoupon
usedCouponId = defaultCoupons?.defaultCoupon?.id
}Note: You'll need to pass unitPrice from the calling code at lines 301-306.
📝 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.
| // this application only supports percentage discounts | |
| // so we only need to check the percentage discount | |
| const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon | |
| if ( | |
| incomingMerchantCoupon?.percentageDiscount && | |
| incomingMerchantCoupon.percentageDiscount >= | |
| defaultMerchantCoupon.percentageDiscount | |
| defaultMerchantCoupon.percentageDiscount | |
| async function getActiveMerchantCoupon({ | |
| productId, | |
| siteCouponId, | |
| code, | |
| unitPrice, | |
| }: { | |
| productId: string | undefined | |
| siteCouponId: string | undefined | |
| code: string | undefined | |
| unitPrice: number | |
| }) { | |
| // … earlier logic … | |
| if ( | |
| incomingCoupon?.merchantCoupon && | |
| couponIsValid(incomingCoupon) && | |
| defaultMerchantCoupon | |
| ) { | |
| // use whichever coupon provides the bigger discount in monetary terms | |
| const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon | |
| const incomingDiscount = | |
| incomingMerchantCoupon.amountDiscount ?? | |
| (incomingMerchantCoupon.percentageDiscount * unitPrice) / 100 | |
| const defaultDiscount = | |
| defaultMerchantCoupon.amountDiscount ?? | |
| (defaultMerchantCoupon.percentageDiscount * unitPrice) / 100 | |
| if (incomingDiscount >= defaultDiscount) { | |
| activeMerchantCoupon = incomingMerchantCoupon | |
| usedCouponId = incomingCoupon.id | |
| } else { | |
| activeMerchantCoupon = defaultMerchantCoupon | |
| usedCouponId = defaultCoupons?.defaultCoupon?.id | |
| } | |
| } | |
| // … later logic … | |
| } |
🤖 Prompt for AI Agents
In apps/course-builder-web/src/trpc/api/routers/pricing.ts around lines 141 to
147, the current guard only checks incomingMerchantCoupon.percentageDiscount
which causes fixed-amount coupons to never win; remove or update the outdated
"only supports percentage discounts" comment, accept a unitPrice parameter
passed from the caller (update the calling site at lines 301-306 to supply
unitPrice), and change the comparison to compute actual monetary savings for
each coupon: if a coupon has amountDiscount use that value, if it has
percentageDiscount compute percentageDiscount * unitPrice, then compare those
numeric savings and pick the coupon that yields the larger monetary discount
(handling missing fields by treating missing discounts as zero).
| // this application only supports percentage discounts | ||
| // so we only need to check the percentage discount | ||
| const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon | ||
| if ( | ||
| incomingMerchantCoupon?.percentageDiscount && | ||
| incomingMerchantCoupon.percentageDiscount >= | ||
| defaultMerchantCoupon.percentageDiscount | ||
| defaultMerchantCoupon.percentageDiscount |
There was a problem hiding this comment.
Critical: Discount comparison logic incomplete for fixed discounts.
The PR objective is to add fixed discount support, but this comparison logic only handles percentage discounts. When incomingMerchantCoupon has a fixed/amount discount (no percentageDiscount), the condition at line 145 will fail, causing the function to always choose defaultMerchantCoupon instead.
To properly support both discount types:
- Calculate the monetary value of both discounts (considering unitPrice for percentage discounts)
- Compare the monetary values
- Choose whichever provides greater savings
The comment on lines 141-142 stating "this application only supports percentage discounts" contradicts the PR's stated purpose.
Consider refactoring to handle both discount types:
- // use whichever coupon provides the bigger discount
- // this application only supports percentage discounts
- // so we only need to check the percentage discount
const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon
- if (
- incomingMerchantCoupon?.percentageDiscount &&
- incomingMerchantCoupon.percentageDiscount >=
- defaultMerchantCoupon.percentageDiscount
- ) {
+
+ // Compare discounts by monetary value to handle both percentage and fixed discounts
+ // This requires the product's unitPrice - fetch it if not already available
+ // Then calculate:
+ // - For percentage: unitPrice * (percentageDiscount / 100)
+ // - For fixed: amountDiscount
+ // Choose the coupon with the larger monetary savings
+
+ const incomingDiscountValue = calculateDiscountValue(incomingMerchantCoupon, unitPrice)
+ const defaultDiscountValue = calculateDiscountValue(defaultMerchantCoupon, unitPrice)
+
+ if (incomingDiscountValue >= defaultDiscountValue) {
activeMerchantCoupon = incomingMerchantCoupon
usedCouponId = incomingCoupon.id
} else {
activeMerchantCoupon = defaultMerchantCoupon
usedCouponId = defaultCoupons?.defaultCoupon?.id
}Note: You'll need to fetch the product's unitPrice within this function and implement a calculateDiscountValue helper that handles both percentageDiscount and amountDiscount fields.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/epicdev-ai/src/trpc/api/routers/pricing.ts around lines 141 to 147, the
current logic only checks percentageDiscount and ignores fixed/amount discounts;
update to compute monetary discount values for both defaultMerchantCoupon and
incomingMerchantCoupon, fetching the product's unitPrice in this function,
implement a calculateDiscountValue helper that returns amountOff =
percentageDiscount ? unitPrice * (percentageDiscount / 100) : amountDiscount,
then compare the resulting monetary values and select the coupon that gives the
larger savings (use the calculated amountOff for comparison instead of only
checking percentageDiscount).
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/epic-web/src/trpc/api/routers/pricing.ts (1)
143-153: Critical: Logic cannot handle fixed amount discounts.The comparison logic only checks
percentageDiscountfields. If the incoming coupon has a fixed amount discount instead of a percentage discount, the condition on line 144 will fail (sinceincomingMerchantCoupon?.percentageDiscountwill be null/undefined), causing the code to always select the default coupon regardless of which provides better value.This breaks the feature described in the PR screenshots showing "fixed coupon chosen over default ($100 > 40% off)". To properly compare a fixed amount discount to a percentage discount, you must calculate their monetary values using the product's unit price.
The logic needs to:
- Check the discount type (
discountType) of each coupon- Calculate the actual monetary discount value for both coupons using the product price
- Compare the monetary values to select the coupon with the greater discount
Consider implementing logic similar to:
// Calculate monetary discount for each coupon const incomingDiscount = calculateDiscount( incomingMerchantCoupon, unitPrice ) const defaultDiscount = calculateDiscount( defaultMerchantCoupon, unitPrice ) if (incomingDiscount >= defaultDiscount) { activeMerchantCoupon = incomingMerchantCoupon usedCouponId = incomingCoupon.id } else { activeMerchantCoupon = defaultMerchantCoupon usedCouponId = defaultCoupons?.defaultCoupon?.id }Where
calculateDiscountcomputes the actual dollar amount based on discount type.apps/astro-party/src/trpc/api/routers/pricing.ts (1)
143-153: Incomplete comparison logic cannot handle fixed discounts.The comparison only checks
percentageDiscountvalues. When the incoming coupon has a fixed amount discount (nopercentageDiscountproperty), the condition on line 144 fails, causing the default coupon to always be selected—even if the fixed discount provides greater savings.According to the PR screenshots:
- "default coupon chosen over fixed coupon (40% > $20 off)"
- "fixed coupon chosen over default ($100 > 40% off)"
These decisions require comparing the actual monetary discount amounts, not just percentage values. To make this comparison, you need:
- The product's unit price
- Logic to compute monetary value:
percentageDiscount * unitPrice / 100vsamountDiscountRetrieve the product's unit price and refactor the comparison to evaluate monetary discount values:
const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon // Fetch product to get unit price for comparison const product = productId ? await db.query.products.findFirst({ where: (products, { eq }) => eq(products.id, productId), }) : null if (!product?.price) { // Fallback: if no product price, prefer percentage discounts if ( incomingMerchantCoupon?.percentageDiscount && incomingMerchantCoupon.percentageDiscount >= (defaultMerchantCoupon.percentageDiscount ?? 0) ) { activeMerchantCoupon = incomingMerchantCoupon usedCouponId = incomingCoupon.id } else { activeMerchantCoupon = defaultMerchantCoupon usedCouponId = defaultCoupons?.defaultCoupon?.id } } else { // Calculate monetary discount for incoming coupon const incomingDiscount = incomingMerchantCoupon?.percentageDiscount ? (product.price * incomingMerchantCoupon.percentageDiscount) / 100 : incomingMerchantCoupon?.amountDiscount ?? 0 // Calculate monetary discount for default coupon const defaultDiscount = defaultMerchantCoupon.percentageDiscount ? (product.price * defaultMerchantCoupon.percentageDiscount) / 100 : defaultMerchantCoupon.amountDiscount ?? 0 if (incomingDiscount >= defaultDiscount) { activeMerchantCoupon = incomingMerchantCoupon usedCouponId = incomingCoupon.id } else { activeMerchantCoupon = defaultMerchantCoupon usedCouponId = defaultCoupons?.defaultCoupon?.id } }apps/cursor-pro/src/trpc/api/routers/pricing.ts (1)
204-207: Runtime error:percentageDiscountmay be undefined with fixed-discount coupons.Line 206 calls
.toString()onpercentageDiscountwithout verifying it exists. With the PR's introduction of fixed (amount) discounts,percentageDiscountmay beundefinedornull, causing a runtime exception.Apply this diff to handle both discount types safely:
const minimalDefaultCoupon = defaultCoupon && { expires: defaultCoupon.expires?.toISOString(), - percentageDiscount: defaultCoupon.percentageDiscount.toString(), + ...(defaultCoupon.percentageDiscount !== undefined && { + percentageDiscount: defaultCoupon.percentageDiscount.toString(), + }), + ...(defaultCoupon.amountDiscount !== undefined && { + amountDiscount: defaultCoupon.amountDiscount.toString(), + }), }
🧹 Nitpick comments (2)
apps/cursor-pro/src/trpc/api/routers/pricing.ts (2)
109-180: Add JSDoc comment for clarity.The
getActiveMerchantCouponfunction lacks documentation. Consider adding a JSDoc comment to describe its purpose, parameters, and return value.As per coding guidelines
Example:
/** * Determines the active merchant coupon by comparing an incoming coupon * with the default/sale coupon and selecting the one with the larger discount. * * @param productId - Optional product ID to fetch default coupons for * @param siteCouponId - Optional incoming coupon ID * @param code - Optional incoming coupon code * @param unitPrice - Unit price for comparing fixed vs percentage discounts * @returns Object containing the active merchant coupon, used coupon ID, and default coupon if applicable */ async function getActiveMerchantCoupon({ productId, siteCouponId, code, unitPrice, }: { productId: string | undefined siteCouponId: string | undefined code: string | undefined unitPrice: number }) { // ... }
137-137: Replace deprecatedcouponIsValidwithvalidateCoupon
couponIsValid(line 94) is deprecated; importvalidateCouponfrom@coursebuilder/core/utils/validate-couponand replace calls at lines 137 and 158 withvalidateCoupon(coupon, productIds).isValid
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/astro-party/src/trpc/api/routers/pricing.ts(1 hunks)apps/craft-of-ui/src/trpc/api/routers/pricing.ts(1 hunks)apps/cursor-pro/src/trpc/api/routers/pricing.ts(1 hunks)apps/epic-web/src/trpc/api/routers/pricing.ts(1 hunks)apps/go-local-first/src/trpc/api/routers/pricing.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Add JS Doc comments to functions and React components
Files:
apps/craft-of-ui/src/trpc/api/routers/pricing.tsapps/go-local-first/src/trpc/api/routers/pricing.tsapps/astro-party/src/trpc/api/routers/pricing.tsapps/cursor-pro/src/trpc/api/routers/pricing.tsapps/epic-web/src/trpc/api/routers/pricing.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use single quotes, no semicolons, tabs (width: 2), and an 80 character line limit for code formatting.
Organize imports in the following order: React → Next → 3rd party → internal.
Do NOT use Object.defineProperty(exports, ...) for re-exports. Use standard export patterns instead to avoid conflicts with framework internals.
Files:
apps/craft-of-ui/src/trpc/api/routers/pricing.tsapps/go-local-first/src/trpc/api/routers/pricing.tsapps/astro-party/src/trpc/api/routers/pricing.tsapps/cursor-pro/src/trpc/api/routers/pricing.tsapps/epic-web/src/trpc/api/routers/pricing.ts
apps/**/*
📄 CodeRabbit inference engine (.cursor/rules/monorepo-setup.mdc)
Apps are located in the /apps directory
Files:
apps/craft-of-ui/src/trpc/api/routers/pricing.tsapps/go-local-first/src/trpc/api/routers/pricing.tsapps/astro-party/src/trpc/api/routers/pricing.tsapps/cursor-pro/src/trpc/api/routers/pricing.tsapps/epic-web/src/trpc/api/routers/pricing.ts
apps/craft-of-ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/craft-of-ui/.cursor/rules/analytics.mdc)
apps/craft-of-ui/**/*.{ts,tsx}: Thetrackfunction in@/utils/analytics.tsis ONLY for tracking learner/customer activity, such as course progress, video watching, exercise completion, purchase activity, and user preferences. DO NOT usetrackfor internal admin actions, content management, system operations, or backend processes.
Uselogfrom@/server/logger.tsfor system operations, content management, error tracking, performance monitoring, and backend processes.
Always include relevant IDs and context in log messages.Always use kebab-case when naming files
Add JSDoc comments to exported methods and React components
apps/craft-of-ui/**/*.{ts,tsx}:logger.ts(located atapps/ai-hero/src/server/logger.ts) is used in a serverside context, but not in client files
Logging should be added where appropriate for server actions and server components
Files:
apps/craft-of-ui/src/trpc/api/routers/pricing.ts
apps/cursor-pro/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/cursor-pro/.cursor/rules/analytics.mdc)
apps/cursor-pro/**/*.{ts,tsx}: Thetrackfunction in@/utils/analytics.tsis ONLY for tracking learner/customer activity (such as course progress, video watching, exercise completion, purchase activity, and user preferences). DO NOT usetrackfor internal admin actions, content management, system operations, or backend processes.
Uselogfrom@/server/logger.tsfor system operations, content management, error tracking, performance monitoring, and backend processes.Always use kebab-case when naming files
Add JSDoc comments to exported methods and React components
apps/cursor-pro/**/*.{ts,tsx}:logger.ts(located atapps/ai-hero/src/server/logger.ts) is used in a serverside context, but not in client files
Logging should be added where appropriate for server actions and server components
Files:
apps/cursor-pro/src/trpc/api/routers/pricing.ts
apps/epic-web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/epic-web/.cursor/rules/analytics.mdc)
apps/epic-web/**/*.{ts,tsx}: Thetrackfunction in@/utils/analytics.tsis ONLY for tracking learner/customer activity (such as course progress, video watching, exercise completion, purchase activity, and user preferences).
DO NOT usetrackfor internal admin actions, content management, system operations, or backend processes.
Uselogfrom@/server/logger.tsfor system operations, content management, error tracking, performance monitoring, and backend processes.
Always include relevant IDs and context in log messages.Always use kebab-case when naming files
Add JSDoc comments to exported methods and React components
apps/epic-web/**/*.{ts,tsx}:logger.ts(located atapps/ai-hero/src/server/logger.ts) is used in a serverside context, but not in client files
Logging should be added where appropriate for server actions and server components
Files:
apps/epic-web/src/trpc/api/routers/pricing.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: typecheck
- GitHub Check: format
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (3)
apps/craft-of-ui/src/trpc/api/routers/pricing.ts (1)
109-117: Function signature doesn't match AI summary claims.The AI summary states: "Extended coupon evaluation flow now accepts pricing context: added requirements for unitPrice and quantity in getActiveMerchantCoupon." However, the function signature does not include
unitPriceorquantityparameters. This is inconsistent with the summary and would prevent dollar-based discount comparisons.If
unitPriceandquantitywere supposed to be added to enable fixed discount comparisons (as the summary claims), they're missing from this implementation. Please verify whether this was an oversight or if the summary is incorrect.apps/go-local-first/src/trpc/api/routers/pricing.ts (1)
140-146: Clarify comments and compare discounts by monetary value
- Comment claims “only supports percentage discounts,” yet fixed-amount coupons are now added—update to “this block only compares percentage discounts” or remove if misleading
- Current logic compares raw
percentageDiscountvalues; to fairly compare fixed vs percentage, calculate $ value:unitPrice * (percentageDiscount/100)vs.amountDiscount- Null-safety guards (
?.and?? 0) are fine, but ensure full dollar-value comparison is implemented elsewhere or hereapps/cursor-pro/src/trpc/api/routers/pricing.ts (1)
182-214: IgnoreunitPricehere; only percentage discounts are compared.The router’s
getActiveMerchantCouponexplicitly supports percentage‐only logic (“this application only supports percentage discounts”), so addingunitPriceisn’t needed.Likely an incorrect or invalid review comment.
| // this application only supports percentage discounts | ||
| // so we only need to check the percentage discount |
There was a problem hiding this comment.
Comment contradicts PR objectives.
The comment states "this application only supports percentage discounts," but the PR explicitly adds fixed (amount) discount support, as evidenced by the screenshots showing "$20 off" and "$100 off" fixed discounts being compared against percentage discounts.
Remove or update these comments to reflect the actual support for both percentage and fixed discounts:
- // this application only supports percentage discounts
- // so we only need to check the percentage discount
+ // compare the monetary value of discounts to determine which provides greater savings📝 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.
| // this application only supports percentage discounts | |
| // so we only need to check the percentage discount | |
| // compare the monetary value of discounts to determine which provides greater savings |
🤖 Prompt for AI Agents
In apps/astro-party/src/trpc/api/routers/pricing.ts around lines 140–141, the
inline comment incorrectly states "this application only supports percentage
discounts" while the PR adds fixed (amount) discount support; replace or update
that comment to accurately state that the application supports both percentage
and fixed (amount) discounts and briefly describe which fields are checked for
each type, and if the surrounding code only inspects percentage values, extend
the validation/branching to also check and handle fixed-amount discounts (e.g.,
check amount fields and compare appropriately) so the comment and code remain
consistent.
There was a problem hiding this comment.
this is intentional. only ai hero has fixed discounts implemented in application code
There was a problem hiding this comment.
@zacjones93, thank you for the clarification! I understand now that the fixed discount feature is only implemented in the ai-hero app, and other apps like astro-party continue to support only percentage discounts. The comment is indeed accurate for this application.
✏️ Learnings added
Learnt from: zacjones93
PR: badass-courses/course-builder#578
File: apps/astro-party/src/trpc/api/routers/pricing.ts:140-141
Timestamp: 2025-10-14T22:05:16.966Z
Learning: The fixed discount feature (amountDiscount support) was only implemented in the ai-hero app. Other apps in the monorepo (astro-party, code-with-antonio, course-builder-web, epicdev-ai, craft-of-ui, cursor-pro, epic-web, go-local-first) still only support percentage discounts in their application code, even though the core pricing infrastructure supports both types.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| // this application only supports percentage discounts | ||
| // so we only need to check the percentage discount | ||
| const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon | ||
| if ( | ||
| incomingMerchantCoupon?.percentageDiscount && | ||
| incomingMerchantCoupon.percentageDiscount >= | ||
| defaultMerchantCoupon.percentageDiscount | ||
| (defaultMerchantCoupon.percentageDiscount ?? 0) |
There was a problem hiding this comment.
Critical inconsistency: Comments and logic contradict PR objectives.
The PR objectives state this adds "fixed coupon discount" support and the AI summary claims it "adds fixed (amount) discount support alongside percentage discounts." However, the added comments explicitly state "this application only supports percentage discounts," and the comparison logic only handles percentageDiscount values.
Issues:
- Line 145: The guard
incomingMerchantCoupon?.percentageDiscount &&would exclude any fixed/amount discount coupons that don't have apercentageDiscountproperty. - Lines 146-147: The comparison only evaluates percentage values, not dollar amounts. To properly compare fixed vs. percentage discounts, you'd need to compute the actual monetary savings using
unitPriceandquantity. - If a user provides a fixed discount coupon (e.g., $100 off), it would be rejected in favor of a default percentage discount, even if the fixed discount provides greater savings.
To support both discount types as the PR claims, the logic needs to:
- Accept
unitPriceandquantityas parameters (as mentioned in the AI summary but not implemented) - Calculate the dollar value of each discount type
- Compare the actual monetary savings, not just percentage values
- Update or remove the misleading comments
Apply this diff to clarify the current limitation:
- // this application only supports percentage discounts
- // so we only need to check the percentage discount
+ // TODO: Add support for fixed/amount discounts
+ // Currently only comparing percentage discountsOr better yet, implement full fixed discount support if that's the PR's intent.
📝 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.
| // this application only supports percentage discounts | |
| // so we only need to check the percentage discount | |
| const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon | |
| if ( | |
| incomingMerchantCoupon?.percentageDiscount && | |
| incomingMerchantCoupon.percentageDiscount >= | |
| defaultMerchantCoupon.percentageDiscount | |
| (defaultMerchantCoupon.percentageDiscount ?? 0) | |
| // TODO: Add support for fixed/amount discounts | |
| // Currently only comparing percentage discounts | |
| const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon | |
| if ( | |
| incomingMerchantCoupon?.percentageDiscount && | |
| incomingMerchantCoupon.percentageDiscount >= | |
| (defaultMerchantCoupon.percentageDiscount ?? 0) |
| // this application only supports percentage discounts | ||
| // so we only need to check the percentage discount | ||
| const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon | ||
| if ( | ||
| incomingMerchantCoupon?.percentageDiscount && | ||
| incomingMerchantCoupon.percentageDiscount >= | ||
| defaultMerchantCoupon.percentageDiscount | ||
| (defaultMerchantCoupon.percentageDiscount ?? 0) |
There was a problem hiding this comment.
🧩 Analysis chain
Fixed-discount coupons are silently ignored; cannot compare discounts without unitPrice.
The guard at line 145 causes fixed-discount coupons (which lack percentageDiscount) to be ignored, even if they provide better value. Additionally, comparing percentage discounts directly (line 146) without the unit price prevents determining which coupon gives the larger absolute discount.
Per the AI summary, other modules in this PR pass unitPrice and quantity into coupon comparisons to handle both fixed and percentage discounts properly. Without that context, this logic cannot choose the optimal coupon when both types are present.
Consider updating getActiveMerchantCoupon to accept unitPrice as a parameter and compare both discount types by their monetary impact:
async function getActiveMerchantCoupon({
productId,
siteCouponId,
code,
+ unitPrice,
}: {
productId: string | undefined
siteCouponId: string | undefined
code: string | undefined
+ unitPrice: number
}) {Then update the comparison logic:
- // this application only supports percentage discounts
- // so we only need to check the percentage discount
const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon
+ // Calculate monetary discount for incoming coupon
+ const incomingDiscount = incomingMerchantCoupon?.percentageDiscount
+ ? unitPrice * (incomingMerchantCoupon.percentageDiscount / 100)
+ : incomingMerchantCoupon?.amountDiscount ?? 0
+ // Calculate monetary discount for default coupon
+ const defaultDiscount = defaultMerchantCoupon.percentageDiscount
+ ? unitPrice * (defaultMerchantCoupon.percentageDiscount / 100)
+ : defaultMerchantCoupon.amountDiscount ?? 0
+
- if (
- incomingMerchantCoupon?.percentageDiscount &&
- incomingMerchantCoupon.percentageDiscount >=
- (defaultMerchantCoupon.percentageDiscount ?? 0)
- ) {
+ if (incomingDiscount >= defaultDiscount) {
activeMerchantCoupon = incomingMerchantCoupon
usedCouponId = incomingCoupon.id
} else {Run the following script to verify how other pricing routers in this PR handle the comparison:
🏁 Script executed:
#!/bin/bash
# Description: Check how other apps handle coupon comparison with unitPrice
# Search for getActiveMerchantCoupon implementations that use unitPrice
rg -n -A 20 'async function getActiveMerchantCoupon' --type ts
# Search for coupon comparison logic that uses amountDiscount or unitPrice
rg -n -C 5 'amountDiscount|unitPrice.*coupon|coupon.*unitPrice' --type ts -g 'apps/**/pricing*.ts'Length of output: 18470
Add unitPrice/quantity parameters and compare discounts by monetary value
The current guard ignores fixed-amount coupons and can’t determine which coupon yields a larger discount. Mirror the pattern in apps/ai-hero/src/trpc/api/routers/pricing.ts: accept unitPrice and quantity, calculate incomingDiscountAmount and defaultDiscountAmount (using amountDiscount when present, otherwise percentageDiscount × subtotal), then choose the coupon with the higher dollar value.
🤖 Prompt for AI Agents
In apps/cursor-pro/src/trpc/api/routers/pricing.ts around lines 141 to 147, the
guard only checks percentage discounts and ignores fixed-amount coupons; update
the logic to accept unitPrice and quantity, compute subtotal = unitPrice *
quantity, then compute incomingDiscountAmount =
incomingMerchantCoupon.amountDiscount (if present) otherwise
incomingMerchantCoupon.percentageDiscount * subtotal, and likewise compute
defaultDiscountAmount from defaultMerchantCoupon (use amountDiscount if present
otherwise percentageDiscount * subtotal); finally compare the two monetary
amounts and choose the coupon that yields the larger dollar discount.
| // this application only supports percentage discounts | ||
| // so we only need to check the percentage discount | ||
| const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon |
There was a problem hiding this comment.
Comments contradict the PR's purpose of adding fixed discount support.
The comments claim "this application only supports percentage discounts," but the PR summary explicitly states this PR "Adds fixed (amount) discount support alongside percentage discounts." This is a direct contradiction.
If this router truly doesn't support fixed discounts yet, then this represents incomplete implementation of the feature across the codebase.
Remove or update these comments to reflect the actual discount types supported:
- // this application only supports percentage discounts
- // so we only need to check the percentage discount
+ // Compare percentage discounts to determine which coupon provides more value
+ // Note: Fixed amount discount comparison is not yet implemented in this router📝 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.
| // this application only supports percentage discounts | |
| // so we only need to check the percentage discount | |
| const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon | |
| // Compare percentage discounts to determine which coupon provides more value | |
| // Note: Fixed amount discount comparison is not yet implemented in this router | |
| const { merchantCoupon: incomingMerchantCoupon } = incomingCoupon |
🤖 Prompt for AI Agents
In apps/epic-web/src/trpc/api/routers/pricing.ts around lines 140 to 142, the
inline comment incorrectly states "this application only supports percentage
discounts" which contradicts the PR adding fixed-amount discount support; update
or remove that comment to accurately describe supported discount types (e.g.,
"supports percentage and fixed amount discounts") and, if the surrounding logic
only checks percentage fields, expand the comment to note that fixed discounts
are handled elsewhere or update the logic to also validate
incomingMerchantCoupon.fixedAmount as needed so comments and implementation
remain consistent.
4 participants
default coupon chosen over fixed coupon (40% > $20 off)
fixed coupon chosen over default ($100 > 40% off)
fixed coupon admin panel
Summary by CodeRabbit
New Features
UX
Checkout
Documentation
Tests