Skip to content

Apply discount codes at checkout - #26

Open
sergical wants to merge 1 commit into
releasefrom
discount-codes-2
Open

sergical wants to merge 1 commit into
releasefrom
discount-codes-2

Conversation

@sergical

Copy link
Copy Markdown
Member

Customers can enter a discount code at checkout. cartTotal now applies the code's percent, and checkoutTotal passes the cart's code through.

Created with Claude Code

Co-authored-by: Claude <claude@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 849746d. Configure here.


export function checkoutTotal(cart: Cart): number {
const subtotal = cartTotal(cart.items);
const subtotal = cartTotal(cart.items, cart.discount?.percent);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discount applied twice at checkout

High Severity

checkoutTotal now passes the cart's discount into cartTotal, which applies it to the item subtotal, but still applies the same percent again to the final amount. One code therefore compounds, so checkout charges less than the intended discounted price.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 849746d. Configure here.

@github-actions

Copy link
Copy Markdown

Verdict: Request changes — the discount is applied twice, producing incorrect checkout totals.

Correctness (high severity)

  1. Double discount application. checkoutTotal now computes subtotal = cartTotal(cart.items, cart.discount?.percent), which already applies the discount inside cartTotal. But further down, the pre-existing code still runs:

    if (cart.discount) {
      total = applyDiscount(total, cart.discount.percent);
    }

    This re-applies the same discount percent to the post-tax/shipping total. Net effect: a cart with a 10% discount effectively gets ~19% off (1 - (1-0.1)^2), not 10%. This is a real, customer-facing bug. Either remove the second applyDiscount call now that cartTotal handles it, or remove the new parameter from cartTotal and keep the single application — but not both.

  2. Free-shipping threshold now computed on a discounted amount. subtotal returned from cartTotal is now post-discount, so const freeShipping = subtotal >= 100 compares the discounted value against $100, whereas previously it compared the full pre-discount subtotal. This silently changes free-shipping eligibility for discounted carts and doesn't appear to be an intended part of this change — worth confirming with product/business rules, since it also interacts with the double-discount bug above.

  3. Implicit undefined → default parameter reliance. cart.discount?.percent evaluates to undefined when there's no discount, which does correctly fall back to the discountPercent = 0 default per JS semantics (default parameters trigger on undefined, whether explicit or omitted) — so this specific case works, but it's fragile/non-obvious to readers. Prefer the explicit cart.discount?.percent ?? 0 at the call site for clarity and to avoid relying on default-parameter semantics implicitly.

  4. No validation on percent. applyDiscount doesn't clamp/validate percent (negative or >1 values silently produce bad results). Not new, but now exercised through an additional path (cartTotal's new parameter), compounding risk if bad data ever reaches it.

Style / clarity (lower severity)

  • Naming inconsistency: the new parameter is discountPercent in cartTotal, while the type/call site use discount.percent. Pick one convention.
  • applyDiscount's parameter is named percent but is actually used as a fraction (total - total * percent, e.g. 0.1 for 10%). A rename to rate/fraction or a doc comment would prevent misuse, especially now that a second call site threads a value into it.
  • Given the discount is now applied in two places, a short comment explaining intent (or removing the duplication) would significantly improve readability — the current state reads like leftover/duplicated logic.

Tests

run_tests passed: 16 passed, 0 failed (2 suites ran). No failing tests currently exercise the double-discount scenario, so this regression doesn't appear to be covered — adding a test for checkoutTotal with a cart that has discount set would have caught issue #1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant