Refine mentoloop application absolute mode #11
Conversation
Co-authored-by: contact.apexaisolutions <[email protected]>
|
Cursor Agent can help with this pull request. Just |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughThis pull request replaces hardcoded discount validation with a database-backed system. It introduces RPC-based validation and usage recording functions, updates the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as createCheckoutSession()
participant PaymentService as validateDiscountCode()
participant Supabase as RPC: validate_discount_code()
participant DB as discount_codes Table
Client->>API: Request with discount code
API->>PaymentService: Validate with code, userId, amount, plan
PaymentService->>Supabase: Call RPC with context
Supabase->>DB: Query & validate atomically
Note over Supabase: Check: exists, active,<br/>date window, limits,<br/>min amount, plan
DB-->>Supabase: Return validation result
Supabase-->>PaymentService: Return enriched DiscountCode
PaymentService->>PaymentService: Compute discountedAmount
PaymentService-->>API: Return DiscountCode + metadata
API->>API: Apply discount to checkout
API->>Supabase: recordDiscountUsage() RPC
Supabase->>DB: Persist usage record
DB-->>Supabase: Return usage ID
Supabase-->>API: Usage recorded
API-->>Client: Checkout session with applied discount
sequenceDiagram
participant User
participant App as App (Frontend)
participant Service as recordDiscountUsage()
participant RPC as record_discount_usage RPC
participant Table as discount_code_usage Table
User->>App: Complete payment
App->>Service: Record usage with full context
Note over Service: Prepare args:<br/>discountCodeId, userId,<br/>paymentAttemptId, amounts,<br/>ipAddress, userAgent
Service->>RPC: Invoke with structured payload
RPC->>Table: Insert usage record atomically
Note over Table: Audit trail created<br/>for fraud detection
Table-->>RPC: Return usage ID
RPC-->>Service: Success
Service-->>App: Usage ID or null
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
❌ Deploy Preview for marvelous-biscuit-5e815c failed. Why did it fail? →
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| discountPercent = validatedDiscount.percentOff; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Bug: Checkout Ignores Fixed Discounts
Fixed-amount discounts aren't applied to the checkout total. The validateDiscountCode function sets percentOff to 0 for these discounts, which means the checkout function's final amount calculation doesn't account for them.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/supabase/services/payments.ts (1)
631-711: Lowercase codes wrongly rejected before normalization.DiscountCodeValidationSchema enforces uppercase, then you normalize. Accept mixed case and transform to uppercase in the schema, or normalize before validation.
Minimal change (outside this block) to discountCodeValidator plus a tiny tweak here:
- const { code } = validated; - const normalizedCode = code.trim().toUpperCase(); + const normalizedCode = validated.code; // already trimmed/uppercased by schemaAnd update the validator itself (earlier in file):
-const discountCodeValidator = z.string().regex( - /^[A-Z0-9]{6,20}$/, - 'Discount code must be 6-20 uppercase alphanumeric characters' -); +const discountCodeValidator = z.string() + .trim() + .regex(/^[A-Za-z0-9]{6,20}$/, 'Discount code must be 6-20 alphanumeric characters') + .transform((s) => s.toUpperCase());This will make the “normalize to uppercase” test pass and still reject short/invalid codes.
🧹 Nitpick comments (4)
DISCOUNT_CODE_IMPLEMENTATION_COMPLETE.md (1)
336-373: Examples: include fixed-amount flow.Add a usage example showing SAVE10 ($10 off) resulting final amount and how discount_percent is stored (null for fixed) to align expectations.
tests/unit/discount-codes.test.ts (1)
490-537: Missing test coverage for checkout applying fixed-amount discounts.Add an integration-style unit test for createStudentCheckoutSession to verify finalAmount reflects fixed_amount discounts and is clamped at >= 0. I can draft it if helpful.
lib/supabase/services/payments.ts (2)
550-553: Stripe API version not pinned.Consider passing apiVersion from env to stabilize behavior across SDK updates:
-const stripe = new Stripe(process.env.STRIPE_SECRET_KEY || ''); +const stripe = new Stripe(process.env.STRIPE_SECRET_KEY || '', { + apiVersion: process.env.STRIPE_API_VERSION as Stripe.LatestApiVersion | undefined, +});Set STRIPE_API_VERSION to your account’s pinned version.
43-56: Allowlist domains via env to avoid hardcoding preview hosts.Move ALLOWED_DOMAINS to env (comma-separated) and parse once at startup. This avoids stale preview entries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
DISCOUNT_CODE_IMPLEMENTATION_COMPLETE.md(1 hunks)lib/supabase/services/payments.ts(5 hunks)tests/unit/discount-codes.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/unit/discount-codes.test.ts (1)
lib/supabase/services/payments.ts (2)
validateDiscountCode(631-711)recordDiscountUsage(720-772)
lib/supabase/services/payments.ts (1)
lib/types/payments.ts (1)
DiscountCode(13-13)
🔍 Remote MCP
I'll search for relevant information to help with reviewing this discount code implementation pull request.## Summary of Relevant Context for PR Review
Based on my research, here are the key findings relevant to reviewing this discount code implementation PR:
Database-Backed Discount Validation Best Practices
Discount codes can carry particular parameters such as redemption limits, expiration dates, and eligibility rules; with recommended security practices including personalized discount restriction rules and time-based validity limits. The PR aligns with this by implementing validation checks for active status, date windows, and per-user usage limits in the PostgreSQL RPC function.
Supabase RPC Security Considerations
Supabase data APIs are designed to work with Postgres Row Level Security (RLS), where you can restrict access using Policies and grant users access to specific database rows or functions based on their authentication token. The PR mentions implementing RLS policies, which is aligned with security best practices.
Security best practices for RPC functions include validating inputs and using prepared statements to prevent SQL injection attacks, and encapsulating business logic within the database minimizes exposure. The atomic checks and validation logic in the validate_discount_code RPC should be reviewed for proper input validation.
RPC functions can be affected by RLS rules, so functions should run with BYPASSRLS roles when appropriate. Reviewers should verify that the RPC functions are configured correctly with appropriate roles to prevent validation bypass issues.
IP Address Tracking for Fraud Detection
IP addresses play a crucial role in fraud detection by providing information about online activity origin; IP addresses are assigned reputation scores based on historical data indicating trustworthiness, and businesses can assess fraud likelihood by analyzing IP address reputation.
IP addresses can be used to analyze user behavior patterns; by tracking activities from a particular IP address, businesses can detect anomalies that may indicate fraudulent behavior, and multiple user accounts from the same IP exhibiting suspicious behavior is a red flag.
However, limitations exist with IP-based fraud detection, as multiple users may share the same IP (public WiFi, corporate environments), and dynamic IP addresses assigned by ISPs can change periodically, requiring fraud systems to incorporate additional techniques and combine multiple data points.
Test Coverage
The PR includes 18 test scenarios covering validation, recording, and integration flows, which is comprehensive. The test suite should verify edge cases around IP/user-agent tracking and ensure proper handling of fraud detection data collection.
🔇 Additional comments (2)
DISCOUNT_CODE_IMPLEMENTATION_COMPLETE.md (1)
209-224: Security posture: clarify RPC execution context and RLS.Please confirm the RPCs run with SECURITY DEFINER and least-privilege roles, and document how RLS is enforced/bypassed per function to prevent validation bypass or data leakage (especially for analytics/stats RPC). I can help add a checklist section here.
tests/unit/discount-codes.test.ts (1)
232-260: Test is correct; implementation currently rejects lowercase codes.This spec (normalize to uppercase) is good. The current validator requires uppercase before normalization, so this will fail until the validator accepts mixed case and transforms. See fix in payments.ts (discountCodeValidator).
| - `createCheckoutSession()`: | ||
| - Calculates base amount before discount validation | ||
| - Passes full validation context (userId, amount, plan) | ||
| - Stores validated discount details for usage recording | ||
|
|
There was a problem hiding this comment.
Doc/code mismatch: fixed-amount discounts not applied in checkout total.
The doc says final amount is computed using the validated discount, but in lib/supabase/services/payments.ts only percentage discounts affect finalAmount; fixed_amount is ignored. Either update the doc or (preferably) apply fixed-amount discounts in createStudentCheckoutSession. See my suggested fix in that file’s comment.
| /** | ||
| * Discount code structure for backward compatibility | ||
| */ | ||
| interface DiscountCode { | ||
| code: string; | ||
| percentOff: number; | ||
| active: boolean; | ||
| expiresAt?: string; | ||
| discountCodeId?: string; | ||
| discountType?: 'percentage' | 'fixed_amount'; | ||
| discountValue?: number; | ||
| currency?: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Type name collision risk with global DiscountCode alias.
This file’s local interface DiscountCode differs from lib/types/payments.ts’ DiscountCode (DB row type). Rename locally to avoid confusion:
-interface DiscountCode {
+interface ValidatedDiscount {
code: string;
percentOff: number;
active: boolean;
expiresAt?: string;
discountCodeId?: string;
discountType?: 'percentage' | 'fixed_amount';
discountValue?: number;
currency?: string;
}And update return types/usages:
-export async function validateDiscountCode(...): Promise<DiscountCode | null>
+export async function validateDiscountCode(...): Promise<ValidatedDiscount | null>
- let validatedDiscount: DiscountCode | null = null;
+ let validatedDiscount: ValidatedDiscount | null = null;Safer for future maintenance.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lib/supabase/services/payments.ts around lines 142 to 154, the local
interface named DiscountCode collides with the global DB row type
lib/types/payments.ts::DiscountCode; rename the local interface to something
unambiguous (e.g., LegacyDiscountCode or DiscountCodeDTO) and update all local
references, function return types and any usages in this file to the new name to
avoid type confusion and ensure correct typing going forward.
| // Validate discount code if provided with full context | ||
| let discountPercent = 0; | ||
| let validatedDiscount: DiscountCode | null = null; | ||
| if (discountCode) { | ||
| validatedDiscount = await validateDiscountCode(supabase, { | ||
| code: discountCode, | ||
| userId, | ||
| amount: baseAmount, | ||
| membershipPlan, | ||
| }); | ||
| if (validatedDiscount) { | ||
| discountPercent = validatedDiscount.percentOff; | ||
| } | ||
| } | ||
|
|
||
| const finalAmount = Math.round(baseAmount * (1 - discountPercent / 100)); |
There was a problem hiding this comment.
Fixed-amount discounts are ignored in finalAmount.
Only percent is applied; fixed_amount should reduce baseAmount. Also clamp to non-negative. Apply this:
- // Validate discount code if provided with full context
- let discountPercent = 0;
- let validatedDiscount: DiscountCode | null = null;
+ // Validate discount code if provided with full context
+ let discountPercent = 0;
+ let discountAmount = 0;
+ let validatedDiscount: DiscountCode | null = null;
if (discountCode) {
validatedDiscount = await validateDiscountCode(supabase, {
code: discountCode,
userId,
amount: baseAmount,
membershipPlan,
});
- if (validatedDiscount) {
- discountPercent = validatedDiscount.percentOff;
- }
+ if (validatedDiscount) {
+ if (validatedDiscount.discountType === 'fixed_amount' && typeof validatedDiscount.discountValue === 'number') {
+ const fixed = Math.max(0, Math.round(validatedDiscount.discountValue));
+ discountAmount = Math.min(baseAmount, fixed);
+ } else {
+ discountPercent = validatedDiscount.percentOff;
+ discountAmount = Math.round(baseAmount * (discountPercent / 100));
+ }
+ }
}
-
- const finalAmount = Math.round(baseAmount * (1 - discountPercent / 100));
+ const finalAmount = Math.max(0, baseAmount - discountAmount);This keeps discount_percent null for fixed-amount (current logic already sets 0 || null => null).
🤖 Prompt for AI Agents
In lib/supabase/services/payments.ts around lines 509 to 524, the code only
applies percent_off and ignores fixed_amount discounts; update the finalAmount
calculation to handle both types: if validatedDiscount has a fixed_amount use
that (capped at baseAmount) as the discount value and keep discountPercent as
null for fixed discounts, otherwise compute discount value from
validatedDiscount.percentOff; then set finalAmount = Math.max(0,
Math.round(baseAmount - discountValue)) to clamp non-negative and handle the
case when validatedDiscount is null.
…application-absolute-mode-1e1a
This pull request contains changes generated by Cursor background composer.
Summary by CodeRabbit
Release Notes
New Features
Tests