Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Greptile Overview
Summary
This PR fixes a bug in the one-time purchase validation logic for stackable offers in the payments system. The main change corrects the `validatePurchaseSession` function to properly respect the `stackable` property of offers when determining whether to allow duplicate purchases.Previously, the code was incorrectly blocking all duplicate one-time purchases regardless of whether the offer was configured as stackable. The fix adds a condition && offer.stackable !== true to the validation check, ensuring that only non-stackable offers prevent duplicate purchases. This aligns the one-time purchase logic with the existing subscription validation pattern already present in the codebase.
The change is accompanied by comprehensive test coverage that validates three key scenarios: allowing duplicate purchases for stackable offers, blocking one-time purchases when subscriptions exist for non-stackable offers, and allowing one-time purchases when subscriptions exist for stackable offers. This ensures the stackable offer feature works correctly across different purchase combinations.
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
| apps/backend/src/lib/payments.tsx | 5/5 | Fixed validation logic to respect stackable property for one-time purchases |
| apps/backend/src/lib/payments.test.tsx | 5/5 | Added comprehensive test coverage for stackable offer validation scenarios |
Confidence score: 5/5
- This PR is safe to merge with minimal risk
- Score reflects a simple, well-tested bug fix that aligns existing inconsistent logic
- No files require special attention
Sequence Diagram
sequenceDiagram
participant User
participant API
participant PaymentService
participant Database
User->>API: "Create Purchase Session"
API->>PaymentService: "validatePurchaseSession()"
PaymentService->>Database: "ensureCustomerExists()"
Database-->>PaymentService: "Customer validation result"
PaymentService->>PaymentService: "Validate offer prices and stackability"
PaymentService->>Database: "findMany() - get existing one-time purchases"
Database-->>PaymentService: "Existing one-time purchases"
PaymentService->>PaymentService: "Check if offer already purchased (if not stackable)"
PaymentService->>PaymentService: "getSubscriptions()"
PaymentService->>Database: "findMany() - get customer subscriptions"
Database-->>PaymentService: "Customer subscriptions"
PaymentService->>PaymentService: "Check subscription conflicts (if not stackable)"
PaymentService->>PaymentService: "Check group conflicts for one-time purchases"
PaymentService->>PaymentService: "Identify conflicting group subscriptions"
PaymentService-->>API: "Validation result with conflicts"
API-->>User: "Purchase session validation response"
2 files reviewed, no comments
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on a914562..7e828ea
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (1)
• apps/backend/src/lib/payments.tsx
⏭️ Files skipped (1)
| Locations |
|---|
apps/backend/src/lib/payments.test.tsx |
- Resolved merge conflicts in apps/backend/src/lib/payments.tsx - Updated terminology from offers/groups to products/catalogs - Fixed TypeScript errors in payments.test.tsx - All linting and type checks pass
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/backend/src/lib/payments.tsx (1)
304-324: Consider explicit destructuring for clarity.The logic correctly handles multiple default products per catalog and throws a descriptive error when misconfigured. However, line 312 uses
productas a tuple, which could be more explicit.Consider this refactor for improved readability:
if (defaultCatalogProducts.length > 0) { - const product = defaultCatalogProducts[0]; + const [productId, productDef] = defaultCatalogProducts[0]; subscriptions.push({ id: null, - productId: product[0], - product: product[1], + productId, + product: productDef, quantity: 1, currentPeriodStart: DEFAULT_PRODUCT_START_DATE, currentPeriodEnd: null, status: SubscriptionStatus.active, createdAt: DEFAULT_PRODUCT_START_DATE, stripeSubscriptionId: null, }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/backend/src/lib/payments.test.tsx(5 hunks)apps/backend/src/lib/payments.tsx(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--purchase-session.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
In tests, prefer .toMatchInlineSnapshot where possible; refer to snapshot-serializer.ts for snapshot formatting and handling of non-deterministic values
Files:
apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--purchase-session.test.tsapps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.tsapps/backend/src/lib/payments.test.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--purchase-session.test.tsapps/backend/src/lib/payments.tsxapps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.tsapps/backend/src/lib/payments.test.tsx
🧬 Code graph analysis (2)
apps/backend/src/lib/payments.tsx (1)
packages/stack-shared/src/utils/errors.tsx (2)
StackAssertionError(69-85)StatusError(152-261)
apps/backend/src/lib/payments.test.tsx (1)
apps/backend/src/lib/payments.tsx (2)
validatePurchaseSession(389-482)getSubscriptions(264-345)
⏰ 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). (10)
- GitHub Check: Vercel Agent Review
- GitHub Check: lint_and_build (latest)
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: all-good
- GitHub Check: docker
- GitHub Check: restart-dev-and-test
- GitHub Check: docker
- GitHub Check: Security Check
🔇 Additional comments (8)
apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts (1)
793-846: LGTM! Test correctly validates non-stackable one-time purchase blocking.The test properly validates that:
- The product is configured as non-stackable (
stackable: falseon line 803)- Duplicate purchases are blocked with status 400 (line 844)
- The error message matches the new unified format (line 845)
These changes align with the PR objective to enforce stackability constraints for one-time purchases.
apps/backend/src/lib/payments.tsx (1)
443-449: LGTM! Consolidated stackability validation is correct.The validation logic correctly:
- Checks both active subscriptions and existing one-time purchases for non-stackable products
- Only validates when a specific
productIdis provided (important for inline products)- Uses the unified error message format consistent across the codebase
The implementation properly addresses the PR objective to allow stackable products to be purchased multiple times while blocking non-stackable products.
apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--purchase-session.test.ts (1)
793-846: LGTM! Legacy API test maintains consistency.The test correctly validates the same non-stackable behavior for the legacy
/api/v1endpoint, ensuring backward compatibility. The changes mirror those in the primary test file and use the unified error message format.apps/backend/src/lib/payments.test.tsx (5)
845-845: LGTM! Error message updated to match unified format.The test expectation correctly reflects the new consolidated error message introduced in the core validation logic.
916-952: LGTM! Test validates stackable product behavior.This test correctly validates that:
- Duplicate purchases are allowed when
product.stackableistrue(line 940)- Quantity > 1 is permitted for stackable products (line 947)
- The validation succeeds without throwing an error
This test directly addresses the PR objective to allow stackable products to be purchased multiple times.
954-1007: LGTM! Test validates subscription blocking for non-stackable products.This test correctly validates that:
- Non-stackable products (line 965) cannot be repurchased when an active subscription exists
- The validation checks subscriptions in addition to one-time purchases
- The unified error message is used (line 1006)
This ensures the consolidated stackability check works correctly across both purchase types.
1009-1065: LGTM! Test validates allowing stackable products with existing subscriptions.This test correctly validates that:
- Stackable products (line 1020) can be purchased multiple times
- Existing subscriptions don't block additional purchases for stackable products
- The validation succeeds without throwing an error
This is the complementary test case to the non-stackable scenario, ensuring complete coverage of the stackability logic.
1121-1121: LGTM! Terminology updates improve clarity.The test descriptions and error messages have been updated to use:
- "products" instead of "offers" (line 1121)
- "catalogs" instead of "groups" (lines 1166, 1205)
These changes align with the terminology used in the core implementation and improve consistency across the codebase.
Also applies to: 1166-1166, 1205-1205
High-level PR Summary
This PR fixes a bug in the one-time purchase validation logic by allowing stackable offers to be purchased multiple times. Previously, the system blocked duplicate one-time purchases regardless of whether the offer was marked as stackable. The fix adds a check to only prevent duplicate purchases when the offer's
stackableproperty is nottrue, enabling customers to purchase stackable offers more than once.⏱️ Estimated Review Time: 15-30 minutes
💡 Review Order Suggestion
apps/backend/src/lib/payments.tsxapps/backend/src/lib/payments.test.tsxImportant
Fixes purchase validation to allow multiple purchases of stackable products and updates tests accordingly.
validatePurchaseSessioninpayments.tsxto allow multiple purchases of stackable products.payments.test.tsxto verify stackable and non-stackable purchase logic.This description was created by
for 113d035. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit