Add new subscription plan: Standard #827
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a tier-aware pricing system for subscription plans. The changes add a PricingTier type (containing credits and monthly_charge) and integrate tier selection and pricing throughout the billing infrastructure. Key additions include: new tier-aware accessor methods in PricingPlanData (get_active_credits, get_active_monthly_charge, get_pricing_title, get_pricing_caption), a STANDARD plan in the PricingPlan enum with multiple tier definitions, updates to Stripe integration to handle tier-specific product IDs and line items, UI rendering functions in billing.py that propagate selected_tier through plan cards and dialogs, and amount/charged_amount fields on the Subscription model. The changes also update webhooks to extract and store tier-related amounts from Stripe subscription metadata and quantity, refactor plan validation in PayPal router for clearer error messages, and include database migrations to support new fields and updated plan choice lists. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
daras_ai_v2/billing.py(3 hunks)payments/auto_recharge.py(1 hunks)payments/migrations/0007_alter_subscription_auto_recharge_enabled.py(1 hunks)payments/models.py(1 hunks)payments/plans.py(1 hunks)scripts/disable_auto_recharge_for_restricted_workspaces.py(1 hunks)workspaces/models.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
payments/auto_recharge.py (1)
workspaces/models.py (1)
allow_credit_topups(223-247)
daras_ai_v2/billing.py (2)
daras_ai_v2/grid_layout_widget.py (1)
grid_layout(25-38)workspaces/models.py (1)
allow_credit_topups(223-247)
workspaces/models.py (2)
app_users/models.py (2)
AppUserTransaction(277-396)TransactionReason(265-274)payments/plans.py (2)
PricingPlan(51-405)from_sub(282-283)
scripts/disable_auto_recharge_for_restricted_workspaces.py (3)
app_users/models.py (2)
AppUserTransaction(277-396)TransactionReason(265-274)payments/models.py (1)
Subscription(37-390)payments/plans.py (1)
PricingPlan(51-405)
🪛 Ruff (0.14.1)
payments/migrations/0007_alter_subscription_auto_recharge_enabled.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-18: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ 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). (2)
- GitHub Check: Analyze (python)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (8)
payments/models.py (1)
48-48: LGTM - Default change aligns with new permission model.The change to
default=Falseforauto_recharge_enabledaligns with the PR's goal to disable auto-recharge for non-Enterprise users without top-up history. The migration handles existing records appropriately.payments/migrations/0007_alter_subscription_auto_recharge_enabled.py (1)
1-18: LGTM - Migration correctly alters field default.The migration properly changes the
auto_recharge_enabledfield default toFalse. The static analysis hints aboutClassVarare false positives - Django migration attributes (dependenciesandoperations) should not be type-annotated as they are framework-specific class attributes.scripts/disable_auto_recharge_for_restricted_workspaces.py (1)
24-91: LGTM - Script correctly implements batch updates for restricted workspaces.The script logic is sound:
- Uses an efficient
Existssubquery to check for top-up history (lines 29-32)- Correctly filters for non-Enterprise subscriptions with auto-recharge enabled and no top-up history (lines 38-49)
- Batch processing with lazy QuerySet evaluation ensures updated records are excluded from subsequent batches (lines 71-91)
The alignment with
workspace.allow_credit_topups()ensures consistency across the codebase.payments/auto_recharge.py (1)
31-38: LGTM - Auto-recharge prerequisite correctly enforces workspace capability.The addition of
workspace.allow_credit_topups()as a prerequisite for auto-recharge ensures that only Enterprise workspaces or those with prior top-up history can use this feature. The placement as the first condition enables short-circuit evaluation for efficiency.daras_ai_v2/billing.py (3)
187-187: LGTM - Simplified to fixed 3-column layout.The change from dynamic column count to a fixed 3-column grid simplifies the rendering logic while maintaining appropriate display for all plans.
524-527: LGTM - Addon section properly gated by workspace capability.The early return when
workspace.allow_credit_topups()isFalsecorrectly hides the entire addon purchase section for workspaces without top-up permissions. This aligns with the auto-recharge prerequisite changes inpayments/auto_recharge.py.
921-924: LGTM - Auto-recharge section properly gated by workspace capability.The guard correctly hides the auto-recharge configuration UI for restricted workspaces. This ensures UI consistency with the backend logic in
payments/auto_recharge.pythat enforces the same capability check.workspaces/models.py (1)
223-247: LGTM - Workspace capability method correctly implements top-up permissions.The
allow_credit_topups()method properly defines workspace-level permissions for credit top-ups:
- Enterprise workspaces always have access ✓
- Workspaces with prior top-up history (ADDON or AUTO_RECHARGE transactions) retain access ✓
- All others are restricted ✓
The implementation efficiently checks the Enterprise case first before querying transaction history, and uses local imports to avoid circular dependencies.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
daras_ai_v2/billing.py(25 hunks)payments/migrations/0008_add_plan_tier_key.py(1 hunks)payments/models.py(3 hunks)payments/plans.py(9 hunks)payments/webhooks.py(3 hunks)recipes/VideoBots.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-09T06:59:25.756Z
Learnt from: nikochiko
PR: GooeyAI/gooey-server#757
File: daras_ai_v2/safety_checker.py:68-71
Timestamp: 2025-08-09T06:59:25.756Z
Learning: In the capture_openai_content_policy_violation context manager in daras_ai_v2/safety_checker.py, only openai.BadRequestError exceptions with specific error codes ("content_policy_violation" or "content_filter") should be converted to UserError. Other OpenAI exceptions like APIStatusError should not be converted to UserError and should be allowed to propagate unchanged.
Applied to files:
recipes/VideoBots.py
🧬 Code graph analysis (5)
payments/webhooks.py (1)
payments/plans.py (2)
PricingPlan(86-540)get_by_stripe_product(377-419)
recipes/VideoBots.py (1)
daras_ai_v2/exceptions.py (1)
UserError(58-65)
payments/models.py (1)
payments/plans.py (4)
PricingPlan(86-540)from_sub(366-367)get_active_credits(67-71)get_active_monthly_charge(73-77)
payments/plans.py (1)
payments/utils.py (1)
make_stripe_recurring_plan(4-31)
daras_ai_v2/billing.py (5)
payments/plans.py (11)
get_active_monthly_charge(73-77)get_active_credits(67-71)label(29-30)PricingPlan(86-540)get_default_tier_key(79-83)get_tier(61-65)get_pricing_title(49-53)get_pricing_caption(55-59)from_sub(366-367)get_stripe_line_item(439-460)supports_stripe(433-434)daras_ai_v2/grid_layout_widget.py (1)
grid_layout(25-38)app_users/models.py (1)
PaymentProvider(85-87)payments/models.py (3)
is_paid(145-146)cancel(157-184)Subscription(37-399)workspaces/models.py (2)
cancel(754-757)allow_credit_topups(223-247)
🪛 Ruff (0.14.2)
payments/webhooks.py
173-175: Create your own exception
(TRY002)
173-175: Avoid specifying long messages outside the exception class
(TRY003)
payments/plans.py
441-441: Avoid specifying long messages outside the exception class
(TRY003)
payments/migrations/0008_add_plan_tier_key.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-39: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
daras_ai_v2/billing.py
510-510: Unused function argument: session
(ARG001)
511-511: Unused function argument: user
(ARG001)
⏰ 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). (1)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (19)
payments/models.py (3)
39-44: LGTM!The
plan_tier_keyfield is properly defined with nullable constraints to maintain backward compatibility for existing subscriptions without tiers.
103-110: LGTM!The tier-aware pricing logic is correctly implemented:
- Plan instance is created once and reused efficiently
- Tier key is properly passed to retrieve tier-specific credits and charges
- Falls back gracefully to default values when tier_key is None
54-54: Default change properly addressed through migration and targeted scripts.Verification confirms the implementation is sound. The default change from
TruetoFalseis applied via migration 0007_alter_subscription_auto_recharge_enabled.py, which only affects new subscription records. Existing subscriptions are preserved as-is, with selective updates handled bydisable_auto_recharge_for_restricted_workspaces.py(only non-Enterprise plans without topup history are modified). Code dependencies are defensive—auto_recharge.py safely checks the flag before use—and users can toggle the setting via the billing UI. No code updates or breaking changes identified.payments/migrations/0008_add_plan_tier_key.py (1)
1-39: LGTM!The migration correctly adds the
plan_tier_keyfield and updates plan choices to include the new STANDARD plan. The structure follows Django migration conventions.Note: The static analysis hints about
ClassVarannotations are false positives - Django migrations are expected to definedependenciesandoperationsas class attributes in this exact format.payments/webhooks.py (2)
168-189: LGTM with tier extraction logic correctly implemented.The code properly:
- Retrieves the product with expanded price information
- Handles the new tuple return from
get_by_stripe_product- Prefers tier_key from product match over metadata
- Falls back gracefully to metadata when product doesn't specify a tier
The error handling is appropriate for missing plan mappings.
267-278: LGTM!The
plan_tier_keyparameter is properly added with a default value ofNonefor backward compatibility, and correctly assigned to the subscription instance.payments/plans.py (6)
23-31: LGTM!The
PricingTierNamedTuple is well-designed with appropriate fields and a cleanlabelproperty for display purposes.
47-83: LGTM!The tier-aware methods are correctly implemented with proper fallback behavior:
get_tier()safely handles None and missing keysget_active_credits()andget_active_monthly_charge()use the walrus operator elegantlyget_default_tier_key()provides a sensible default (first tier)
170-204: LGTM!The STARTER plan updates improve clarity by:
- Renaming to "Free" to better communicate the pricing
- Updating descriptions to reflect current features
- Adding a clear pricing caption about starting credits
These are appropriate marketing/content improvements.
377-419: LGTM!The
get_by_stripe_productmethod correctly:
- Returns a tuple
(plan, tier_key)for consistency- Handles legacy products with backward compatibility
- Matches tiered plans by name and price
- Falls back gracefully when price information is unavailable
- Returns
Nonewhen no match is foundThe tier key extraction logic is sound and aligns with product creation in
get_stripe_product_id.
439-460: LGTM!The method correctly:
- Accepts an optional
tier_keyparameter- Uses
get_active_credits()andget_active_monthly_charge()for tier-aware pricing- Propagates the tier_key to
get_stripe_product_id()for product resolution
472-500: LGTM!The tier-aware product ID resolution:
- Builds appropriate product names including tier labels
- Matches existing products by both name and price
- Creates new products when needed with tier-specific pricing
- Maintains consistency with
get_by_stripe_productmatching logicdaras_ai_v2/billing.py (7)
83-87: LGTM!The current plan display correctly retrieves tier-specific pricing using
get_active_monthly_charge()andget_active_credits().
159-165: LGTM!The plan filtering correctly:
- Excludes deprecated plans
- Restricts STANDARD plan to personal workspaces only (business logic requirement)
This ensures team workspaces are guided toward Business/Enterprise plans.
212-273: LGTM with tier selection UI properly implemented.The tier selection logic:
- Only displays tier dropdown for Stripe subscriptions (line 235)
- Uses current subscription's tier as default when available
- Falls back to plan's default tier for new subscriptions
- Updates pricing display based on selected tier
Note: PayPal subscriptions don't support tier selection, which is consistent with PayPal's simpler subscription model.
301-320: LGTM!The tier change detection correctly:
- Identifies when a different tier is selected within the STANDARD plan
- Delegates to
_render_tier_change_buttonfor upgrade/downgrade UI- Shows "Your Plan" button when the selected tier matches current tier
346-369: LGTM!The STANDARD plan button logic:
- Correctly handles both paid and free plan states
- Always uses "Upgrade" label for consistent UX
- Properly propagates
selected_tier_keythrough the flow
733-738: LGTM!The addon section is correctly gated by
workspace.allow_credit_topups(), which aligns with the PR objective to disable addons for new non-enterprise users.
1144-1146: LGTM!The auto-recharge section is consistently gated using the same
workspace.allow_credit_topups()check as the addon section.
| if is_enterprise: | ||
| gui.write( | ||
| f"As a premium customer, please contact us to setup a managed number" | ||
| "As a premium customer, please contact us to setup a managed number" |
There was a problem hiding this comment.
Fix grammatical error: "setup" → "set up".
When used as a verb phrase, "set up" should be two words. Line 1980 correctly uses "set up" in a similar context. This change maintains consistency across the file.
Apply this diff to fix the grammar:
- "As a premium customer, please contact us to setup a managed number"
+ "As a premium customer, please contact us to set up a managed number"📝 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.
| "As a premium customer, please contact us to setup a managed number" | |
| "As a premium customer, please contact us to set up a managed number" |
🤖 Prompt for AI Agents
In recipes/VideoBots.py around line 1939, the string uses the incorrect
noun-form "setup" instead of the verb phrase "set up"; change "As a premium
customer, please contact us to setup a managed number" to "As a premium
customer, please contact us to set up a managed number" so it matches correct
grammar and the usage on line 1980.
nikochiko
force-pushed
the
subscription-plan-standard
branch
2 times, most recently
from
d1024df to
ac20427
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
daras_ai_v2/billing.py(18 hunks)daras_ai_v2/settings.py(1 hunks)payments/plans.py(11 hunks)payments/webhooks.py(5 hunks)routers/paypal.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
routers/paypal.py (1)
payments/plans.py (3)
PricingPlan(88-542)get_by_key(429-433)supports_paypal(438-439)
daras_ai_v2/billing.py (6)
payments/plans.py (11)
get_active_monthly_charge(76-80)get_active_credits(70-74)get_pricing_title(50-55)label(29-30)PricingPlan(88-542)from_sub(373-376)get_default_tier_key(82-85)get_pricing_caption(57-62)get_tier(64-68)get_stripe_line_item(441-462)supports_stripe(435-436)app_users/models.py (2)
PaymentProvider(85-87)cached_workspaces(242-249)daras_ai_v2/grid_layout_widget.py (1)
grid_layout(25-38)routers/account.py (2)
account_route(99-111)payment_processing_route(38-84)payments/models.py (1)
Subscription(37-399)workspaces/models.py (1)
allow_credit_topups(223-247)
payments/webhooks.py (1)
payments/plans.py (3)
PricingPlan(88-542)get_by_key(429-433)get_by_stripe_product(386-420)
payments/plans.py (1)
payments/utils.py (1)
make_stripe_recurring_plan(4-31)
🪛 Ruff (0.14.2)
payments/plans.py
85-85: Avoid specifying long messages outside the exception class
(TRY003)
403-403: Loop control variable tier_key not used within loop body
Rename unused tier_key to _tier_key
(B007)
433-433: Avoid specifying long messages outside the exception class
(TRY003)
443-443: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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). (1)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (10)
routers/paypal.py (1)
123-129: LGTM! Improved error handling for plan validation.The refactored validation logic provides better separation of concerns with distinct error messages for invalid plan keys versus PayPal support issues, making debugging easier for API consumers.
payments/plans.py (2)
23-86: Well-designed tier-aware pricing structure.The
PricingTierclass and tier-aware accessor methods (get_tier,get_active_credits,get_active_monthly_charge) provide a clean abstraction for handling multi-tier plans while maintaining backward compatibility with single-tier plans through sensible defaults.
396-420: Remove unusedtier_keyunpacking; current return signature is sufficient.The
tier_keyvariable on line 403 is genuinely unused—onlytieris referenced in the loop body. This is not a false positive. Both callers (webhooks.py and migrate_existing_subscriptions.py) obtain tier context elsewhere if needed (webhooks.py retrieves it fromstripe_sub.metadata) and don't require tier information fromget_by_stripe_product.Simplify line 403 to
for tier in plan.tiers.values():to eliminate the dead code.Likely an incorrect or invalid review comment.
daras_ai_v2/settings.py (1)
392-392: LGTM! Constant addition for tier metadata.The new constant follows the existing pattern for Stripe metadata fields and enables tier-aware subscription handling.
payments/webhooks.py (1)
187-195: LGTM! Safe tier_key extraction and propagation.The tier_key extraction uses
.get()with appropriate None handling, and the value is correctly passed through toset_workspace_subscription.daras_ai_v2/billing.py (5)
81-142: LGTM! Tier-aware current plan rendering.The tier_key extraction from the subscription and tier-aware pricing display using
get_active_monthly_chargeandget_active_creditscorrectly handles both tiered and non-tiered plans.
154-219: Well-structured tier-aware plan rendering.The filtering of STANDARD plan for non-personal workspaces (lines 157-158) and the separation of grid vs. full-width plans provides good UX differentiation. The tier context is properly propagated through the rendering pipeline.
626-631: LGTM! Appropriate gating of credit topups.The
allow_credit_topups()check correctly hides the addon section for workspaces that shouldn't have access to credit purchases, aligning with the PR objective to disable addons for new non-enterprise users.
768-870: Tier-aware subscription creation logic looks solid.The tier_key is correctly propagated through Stripe metadata and line items, and the pricing calculations use
get_active_monthly_chargeandget_active_creditsto handle tier-specific values.
1033-1039: LGTM! Consistent gating of auto-recharge feature.The
allow_credit_topups()check correctly hides the auto-recharge section, consistent with the addon gating and the PR objective to disable auto-recharge for new non-enterprise users.
| try: | ||
| plan = PricingPlan.get_by_key( | ||
| stripe_sub.metadata[settings.STRIPE_USER_SUBSCRIPTION_METADATA_FIELD] | ||
| ) | ||
| except KeyError: | ||
| product = stripe.Product.retrieve( | ||
| stripe_sub.plan.product, expand=["default_price"] | ||
| ) | ||
| plan = PricingPlan.get_by_stripe_product(product) | ||
| assert plan is not None, f"Plan for product {product.id} not found" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the assertion handles all Stripe subscription scenarios.
The fallback logic asserts that a plan is found when metadata is missing, but this could fail if Stripe contains legacy subscriptions or products that don't match current plan definitions. Consider logging a warning and handling the None case more gracefully.
🏁 Script executed:
#!/bin/bash
# Check if there are other places where missing plan lookups are handled
rg -n -B3 -A3 'get_by_stripe_product|get_by_paypal_plan_id' --type=pyLength of output: 2856
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the webhook handler function and surrounding error handling
rg -n -B10 "def.*webhook" payments/webhooks.py | head -100
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there's try-catch wrapping around the get_by_stripe_product call
rg -n -B10 -A5 "stripe_sub.plan.product" payments/webhooks.pyLength of output: 1046
🏁 Script executed:
#!/bin/bash
# Search for any custom exception handling or logging patterns in webhooks
rg -n "except|logger|logging" payments/webhooks.py | head -40Length of output: 444
---
🏁 Script executed:
```shell
#!/bin/bash
# Find where handle_subscription_updated is called and any wrapping error handlers
rg -n "handle_subscription_updated" --type=py
Length of output: 727
🏁 Script executed:
#!/bin/bash
# Check if there's a Flask/Django error handler or middleware
rg -n "error_handler|errorhandler|middleware|@app|@route" payments/webhooks.py | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look at the start of the file to understand the context (imports, decorators, flask/django setup)
head -50 payments/webhooks.pyLength of output: 1822
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the Stripe webhook endpoint for error handling
rg -n -B5 -A10 "handle_subscription_updated" routers/stripe.py
Length of output: 582
🏁 Script executed:
#!/bin/bash
# Check the PayPal webhook endpoint for error handling
rg -n -B5 -A10 "handle_subscription_updated" routers/paypal.pyLength of output: 705
🏁 Script executed:
#!/bin/bash
# Check if there's a catch-all error handler in the Django app
fd -e py -x rg -l "exception_handler|handle_exception|catch_all|@app" {} \; routers/ | head -20Length of output: 337
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full function context from stripe router
rg -n -B20 "handle_subscription_updated" routers/stripe.py | head -80
Length of output: 932
🏁 Script executed:
#!/bin/bash
# Get full function context from paypal router
rg -n -B20 "handle_subscription_updated" routers/paypal.py | head -80Length of output: 1136
🏁 Script executed:
#!/bin/bash
# Check for exception handlers in main app config
fd . routers/ --name '*.py' -x grep -l "exception_handler\|@app\|def.*webhook" {} \;Length of output: 232
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the full Stripe webhook function definition
rg -n -B40 "handle_subscription_updated" routers/stripe.py | grep -A40 "def webhook"
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if Stripe webhook has any try-catch wrapping the match statement
rg -n "def webhook" routers/stripe.py -A50Length of output: 1999
🏁 Script executed:
#!/bin/bash
# Look for any global exception handlers or middleware in FastAPI setup
rg -n "exception_handler|ExceptionMiddleware|@app.exception_handler" --type=pyLength of output: 498
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the global exception handler implementation
rg -n -A10 "async def server_error_exception_handler" server.py
Length of output: 460
🏁 Script executed:
#!/bin/bash
# Verify that AssertionError will be caught by the Exception handler
rg -n -B5 -A15 "def server_error_exception_handler" server.pyLength of output: 890
Add graceful error handling for missing plan lookups in Stripe webhook handler.
The assertion at line 179 will crash the webhook on legacy subscriptions or products without matching plan definitions. Unlike the migration script which gracefully logs and skips missing plans (lines 115-117 in scripts/migrate_existing_subscriptions.py), the webhook crashes with a 5xx error that Stripe will retry indefinitely.
Replace the assertion with logging and early return:
plan = PricingPlan.get_by_stripe_product(product)
if plan is None:
logger.warning(f"Plan not found for Stripe product {product.id}")
return # Acknowledge webhook to prevent retriesAlso apply the same fix to PayPal webhook handlers at lines 33-34 and 62-63, which have identical assertion patterns without logging.
🤖 Prompt for AI Agents
In payments/webhooks.py around lines 170 to 179 the code asserts that a
PricingPlan exists for a Stripe product which will raise an exception and crash
the webhook for legacy/missing plans; replace the assertion with a null-check
that logs a warning and returns early to acknowledge the webhook (e.g., if plan
is None: logger.warning(f"Plan not found for Stripe product {product.id}");
return). Also apply the same change to the PayPal webhook handlers at lines
~33-34 and ~62-63: remove the assertions, add a warning log indicating the
missing plan with the product/subscription id, and return early so the webhook
responds 200 and Stripe/PayPal will not retry.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
daras_ai_v2/billing.py (1)
384-390: Add defensive checks for tier lookups to prevent AttributeError.Lines 388-390 directly access
.monthly_chargeafterget_tier(), which returnsPricingTier | None. If eitherselected_tier_keyorcurrent_tier_keyis invalid (e.g., from stale data or migration issues), this will raise anAttributeError.Based on the past review comment, this was supposed to be addressed but the defensive handling is still missing.
Apply this diff to add defensive handling:
if plan > current_plan or ( plan == current_plan and plan.tiers + and (current_tier := plan.get_tier(current_tier_key)) + and (selected_tier := plan.get_tier(selected_tier_key)) and ( - plan.get_tier(selected_tier_key).monthly_charge - > plan.get_tier(current_tier_key).monthly_charge + selected_tier.monthly_charge > current_tier.monthly_charge ) ):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
daras_ai_v2/billing.py(17 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
daras_ai_v2/billing.py (6)
payments/plans.py (10)
get_active_monthly_charge(76-80)get_active_credits(70-74)get_pricing_title(50-55)PricingPlan(88-542)from_sub(373-376)get_default_tier_key(82-85)get_pricing_caption(57-62)get_tier(64-68)get_stripe_line_item(441-462)supports_stripe(435-436)app_users/models.py (1)
PaymentProvider(85-87)daras_ai_v2/grid_layout_widget.py (1)
grid_layout(25-38)routers/account.py (2)
account_route(99-111)payment_processing_route(38-84)payments/models.py (1)
Subscription(37-399)workspaces/models.py (1)
allow_credit_topups(223-247)
🔇 Additional comments (12)
daras_ai_v2/billing.py (12)
81-142: LGTM: Tier-aware pricing display is correctly implemented.The function properly retrieves the tier key from the subscription and consistently uses tier-aware accessors (
get_active_monthly_charge,get_active_credits,get_pricing_title) to display pricing information.
168-168: Verify the default payment provider change.The default payment provider has been changed to
PaymentProvider.STRIPEfor workspaces without an existing subscription. Ensure this aligns with your product requirements, especially if PayPal was previously the default or if this impacts user experience in specific regions.
243-275: LGTM: Tier selector logic is well-structured.The function correctly:
- Displays tier selector only when plan has tiers and Stripe is the payment provider
- Preserves current tier selection for existing subscriptions
- Falls back to plan defaults appropriately
- Returns the selected tier key for downstream use
278-338: LGTM: Action button logic correctly handles tier changes.The function properly distinguishes between plan changes and tier changes within the same plan, routing to appropriate handlers with the selected tier key.
422-487: LGTM: Upgrade flow correctly handles tier-aware pricing and STANDARD plan specifics.The function properly:
- Uses safe tier-aware accessors (
get_active_monthly_charge,get_active_credits) that handle None gracefully- Distinguishes STANDARD plan from other plans for workspace creation flow
- Threads tier context through upgrade and fallback paths
489-523: LGTM: Create subscription flow handles plan-specific behavior correctly.The function appropriately:
- Differentiates UI labels for STANDARD vs other plans
- Routes personal workspace users away from workspace creation for STANDARD plan
- Passes tier context to Stripe (PayPal doesn't support tiers)
525-529: LGTM: Price formatting helper is simple and correct.
532-600: LGTM: Subscription change flow properly integrates tier context.The function correctly:
- Validates that plan or tier has actually changed before proceeding
- Includes tier information in Stripe metadata using the configured metadata field
- Passes tier context through to line item generation
- Appropriately omits tier handling for PayPal (which doesn't support tiers)
614-636: LGTM: Addon section correctly gated by workspace capability.The early return at lines 618-619 appropriately hides the entire addon purchase section for workspaces that don't have credit topup privileges (new non-Enterprise users without topup history).
756-801: LGTM: Stripe subscription button correctly handles tier-aware pricing.The function properly retrieves tier-specific pricing and passes the tier context through all subscription creation paths.
803-858: LGTM: Stripe subscription creation consistently includes tier context.The function properly:
- Conditionally adds tier to metadata when provided
- Uses configured metadata field name for consistency
- Passes tier context to line item generation
- Includes metadata in both direct subscription creation and checkout flows
1021-1103: LGTM: Auto-recharge section correctly gated by workspace capability.The early return at lines 1026-1027 appropriately hides the auto-recharge configuration section for workspaces without topup privileges, consistent with the addon section gating.
| def _render_plan_details(plan: PricingPlan): | ||
| with gui.div(className="flex-grow-1 d-flex flex-column"): | ||
| with gui.div(): | ||
| with gui.tag("h2", className="mb-1"): | ||
| gui.html(plan.title) | ||
| gui.caption( | ||
| plan.description, | ||
| style={ | ||
| "minHeight": "calc(var(--bs-body-line-height) * 2em)", | ||
| "display": "block", | ||
| }, | ||
| """Render plan details and return selected tier key if plan has tiers""" | ||
| with gui.div(): | ||
| gui.write(plan.long_description, unsafe_allow_html=True) | ||
| with gui.div(className="mt-3"): | ||
| gui.write(plan.footer, unsafe_allow_html=True) | ||
|
|
There was a problem hiding this comment.
Fix misleading docstring.
The docstring claims the function returns a selected tier key, but the function doesn't return anything. The tier selection logic was moved to _render_plan_pricing.
Apply this diff to correct the docstring:
-def _render_plan_details(plan: PricingPlan):
- """Render plan details and return selected tier key if plan has tiers"""
+def _render_plan_details(plan: PricingPlan):
+ """Render plan description and footer"""📝 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.
| def _render_plan_details(plan: PricingPlan): | |
| with gui.div(className="flex-grow-1 d-flex flex-column"): | |
| with gui.div(): | |
| with gui.tag("h2", className="mb-1"): | |
| gui.html(plan.title) | |
| gui.caption( | |
| plan.description, | |
| style={ | |
| "minHeight": "calc(var(--bs-body-line-height) * 2em)", | |
| "display": "block", | |
| }, | |
| """Render plan details and return selected tier key if plan has tiers""" | |
| with gui.div(): | |
| gui.write(plan.long_description, unsafe_allow_html=True) | |
| with gui.div(className="mt-3"): | |
| gui.write(plan.footer, unsafe_allow_html=True) | |
| def _render_plan_details(plan: PricingPlan): | |
| """Render plan description and footer""" | |
| with gui.div(): | |
| gui.write(plan.long_description, unsafe_allow_html=True) | |
| with gui.div(className="mt-3"): | |
| gui.write(plan.footer, unsafe_allow_html=True) |
🤖 Prompt for AI Agents
In daras_ai_v2/billing.py around lines 222 to 228, the function
_render_plan_details has a misleading docstring that says it "return[s] selected
tier key if plan has tiers" even though it returns nothing (tier selection was
moved to _render_plan_pricing); update the docstring to accurately describe
behavior (e.g., "Render plan details" or "Render plan details; this does not
return a tier key — tier selection is handled in _render_plan_pricing") so the
docstring matches the function's actual behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
daras_ai_v2/billing.py (2)
222-228: Fix misleading docstring.The docstring claims the function returns a selected tier key, but the function doesn't return anything. The tier selection logic was moved to
_render_plan_pricing.Apply this diff to correct the docstring:
def _render_plan_details(plan: PricingPlan): - """Render plan details and return selected tier key if plan has tiers""" + """Render plan description and footer"""
384-390: Add defensive check for tier lookup in comparison logic.Lines 388-390 directly access
.monthly_chargeafterget_tier(), which returnsPricingTier | None. If the subscription contains an invalidplan_tier_key(e.g., from database migration or stale data), this will raise anAttributeError.Apply this diff to add defensive handling:
if plan > current_plan or ( plan == current_plan and plan.tiers + and (current_tier := plan.get_tier(current_tier_key)) + and (selected_tier := plan.get_tier(selected_tier_key)) and ( - plan.get_tier(selected_tier_key).monthly_charge - > plan.get_tier(current_tier_key).monthly_charge + selected_tier.monthly_charge > current_tier.monthly_charge ) ):
🧹 Nitpick comments (1)
daras_ai_v2/billing.py (1)
433-433: Consider clarifying comment about workspace creation popup logic.The comments state "Standard plan is only for personal workspaces, skip workspace creation popup," but the condition
workspace.is_personal and plan != PricingPlan.STANDARDshows the workspace creation popup for non-Standard plans. The comment could more clearly explain that the popup is shown for personal workspaces upgrading to non-Standard plans (Business, Enterprise).Consider this clearer wording:
- # Standard plan is only for personal workspaces, skip workspace creation popup + # For personal workspaces upgrading to non-Standard plans, show workspace creation popup if workspace.is_personal and plan != PricingPlan.STANDARD:Also applies to: 495-495
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
daras_ai_v2/billing.py(17 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
daras_ai_v2/billing.py (6)
payments/plans.py (10)
get_active_monthly_charge(76-80)get_active_credits(70-74)get_pricing_title(50-55)PricingPlan(88-542)from_sub(373-376)get_default_tier_key(82-85)get_pricing_caption(57-62)get_tier(64-68)get_stripe_line_item(441-462)supports_stripe(435-436)app_users/models.py (1)
PaymentProvider(85-87)daras_ai_v2/grid_layout_widget.py (1)
grid_layout(25-38)routers/account.py (2)
account_route(99-111)payment_processing_route(38-84)payments/models.py (1)
Subscription(37-399)workspaces/models.py (1)
allow_credit_topups(223-247)
⏰ 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). (2)
- GitHub Check: test (3.10.12, 1.8.3)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (9)
daras_ai_v2/billing.py (9)
83-87: LGTM!The tier-aware pricing retrieval is correctly implemented. The
get_active_monthly_chargeandget_active_creditsmethods properly handleNonetier keys and fall back to default values.
230-240: LGTM!Good refactoring to extract the plan heading into a separate function. The consistent minimum height ensures proper grid alignment across plan cards.
243-275: LGTM!The tier selection logic is well-structured:
- Properly defaults to the subscription's current tier when viewing the active plan
- Falls back to the plan's default tier for new plan selections
- Correctly restricts tier selection to Stripe payment provider only
610-612: LGTM!The guard properly restricts addon purchases to Enterprise workspaces or those with existing topup history, aligning with the PR objective to disable addons for new non-enterprise users.
749-793: LGTM!The Stripe subscription button correctly propagates tier information through the payment flow and displays tier-specific pricing in the confirmation modal.
796-851: LGTM!The Stripe subscription creation properly:
- Includes tier information in metadata for webhook processing
- Uses tier-aware line items from
get_stripe_line_item(tier_key)- Propagates metadata through both direct subscription and checkout flows
1018-1020: LGTM!The guard consistently restricts auto-recharge configuration to Enterprise workspaces or those with existing topup history, matching the addon section's behavior and fulfilling the PR objectives.
525-593: LGTM!The subscription change logic properly:
- Checks if both plan and tier are unchanged to prevent no-op updates
- Includes tier information in Stripe metadata
- Uses tier-aware line items
- Correctly handles the Stripe-only tier support (PayPal branch unchanged as expected)
518-522: All callers offmt_priceare properly updated.The verification found two call sites (lines 407 and 455), both passing integer arguments (
current_monthly_chargeandnew_monthly_charge) that match the new signaturefmt_price(monthly_charge: int) -> str. No incompatible calls were found.
nikochiko
force-pushed
the
subscription-plan-standard
branch
from
a31142d to
a6a26f1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
daras_ai_v2/billing.py (2)
282-288: Fix misleading docstring.The docstring still incorrectly states the function returns a selected tier key, but the function returns
None. Tier selection was moved to_render_plan_pricing.Apply this diff to correct the docstring:
def _render_plan_details(plan: PricingPlan): - """Render plan details and return selected tier key if plan has tiers""" + """Render plan description and footer"""
444-451: Add defensive check for tier lookup in comparison logic.Lines 448-449 directly access
.monthly_chargeafterget_tier(), which returnsPricingTier | None. If the subscription contains an invalidcurrent_tier_keyorselected_tier_key, this will raise anAttributeError.Apply this diff to add defensive handling:
if plan > current_plan or ( plan == current_plan and plan.tiers + and (current_tier := plan.get_tier(current_tier_key)) + and (selected_tier := plan.get_tier(selected_tier_key)) and ( - plan.get_tier(selected_tier_key).monthly_charge - > plan.get_tier(current_tier_key).monthly_charge + selected_tier.monthly_charge > current_tier.monthly_charge ) ):
🧹 Nitpick comments (1)
payments/plans.py (1)
382-416: Consider minor loop optimization.The loop variable
tier_keyat line 399 is not used in the loop body. Since only thetiervalues are needed, consider using.values()instead of.items()for clarity.Apply this diff if you'd like to simplify:
- for tier_key, tier in plan.tiers.items(): + for tier in plan.tiers.values(): expected_name = f"{plan.title} - {tier.label}"Otherwise, the tier-matching logic correctly handles both tiered and non-tiered plans.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app_users/migrations/0029_alter_appusertransaction_plan.py(1 hunks)daras_ai_v2/billing.py(17 hunks)payments/migrations/0009_alter_subscription_plan.py(1 hunks)payments/plans.py(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
app_users/migrations/0029_alter_appusertransaction_plan.py (2)
payments/migrations/0009_alter_subscription_plan.py (1)
Migration(6-29)app_users/migrations/0026_alter_appusertransaction_plan.py (1)
Migration(6-18)
payments/plans.py (1)
payments/utils.py (1)
make_stripe_recurring_plan(4-31)
daras_ai_v2/billing.py (5)
payments/plans.py (10)
get_active_monthly_charge(76-80)get_active_credits(70-74)get_pricing_title(50-55)PricingPlan(88-538)from_sub(369-372)get_default_tier_key(82-85)get_pricing_caption(57-62)get_tier(64-68)get_stripe_line_item(437-458)supports_stripe(431-432)app_users/models.py (1)
PaymentProvider(85-87)daras_ai_v2/grid_layout_widget.py (1)
grid_layout(25-38)payments/models.py (1)
Subscription(37-399)workspaces/models.py (1)
allow_credit_topups(223-247)
payments/migrations/0009_alter_subscription_plan.py (3)
app_users/migrations/0029_alter_appusertransaction_plan.py (1)
Migration(6-33)payments/migrations/0005_alter_subscription_plan.py (1)
Migration(6-18)payments/migrations/0006_alter_subscription_plan.py (1)
Migration(6-18)
🪛 Ruff (0.14.2)
app_users/migrations/0029_alter_appusertransaction_plan.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-33: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
payments/plans.py
85-85: Avoid specifying long messages outside the exception class
(TRY003)
399-399: Loop control variable tier_key not used within loop body
Rename unused tier_key to _tier_key
(B007)
429-429: Avoid specifying long messages outside the exception class
(TRY003)
439-439: Avoid specifying long messages outside the exception class
(TRY003)
payments/migrations/0009_alter_subscription_plan.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-29: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ 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). (3)
- GitHub Check: test (3.10.12, 1.8.3)
- GitHub Check: Analyze (python)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (13)
app_users/migrations/0029_alter_appusertransaction_plan.py (1)
1-33: LGTM! Migration correctly adds Standard plan.The migration properly updates the
AppUserTransaction.planfield choices to include the new Standard plan (8) and renames "Pay as you go" to "Free" (3), aligning with the updatedPricingPlanenum.payments/migrations/0009_alter_subscription_plan.py (1)
1-29: LGTM! Migration correctly updates Subscription plan choices.The migration properly adds the Standard plan (8) to the
Subscription.planfield choices and depends on the correct prior migration (0008_add_plan_tier_key), maintaining proper migration sequencing for the tier-aware pricing feature.daras_ai_v2/billing.py (5)
82-143: LGTM! Tier-aware current plan rendering.The function correctly retrieves the
tier_keyfrom the subscription and uses tier-aware pricing methods to display the appropriate monthly charge and credits. The tier-aware getters handleNonevalues safely.
152-192: LGTM! Correct STANDARD plan filtering for workspace types.The filtering logic correctly hides the STANDARD plan from non-personal workspaces (except when already subscribed to it), aligning with the business requirement that STANDARD is designed for personal workspaces. The new compact/full-width rendering structure with tier-aware pricing is well-implemented.
667-688: LGTM! Proper guard for addon section.The early return correctly prevents non-eligible workspaces from accessing the credit top-up functionality, aligning with the PR objective to disable auto-recharge/addons for new non-enterprise users. The check using
workspace.allow_credit_topups()ensures only Enterprise plans or workspaces with prior top-up history can access this section.
1074-1157: LGTM! Proper guard for auto-recharge section.The early return at lines 1079-1080 correctly prevents non-eligible workspaces from accessing auto-recharge functionality, consistent with the addon section guard and aligned with the PR objectives.
809-911: LGTM! Comprehensive tier_key propagation in Stripe subscription flow.The tier_key is correctly threaded through the Stripe subscription creation flow:
- Metadata properly includes tier information via
STRIPE_USER_SUBSCRIPTION_TIER_METADATA_FIELD- Line items use tier-aware pricing via
get_stripe_line_item(tier_key)- Tier-specific credits and monthly charges are displayed in the UI
payments/plans.py (6)
23-31: LGTM! Clean PricingTier type definition.The
PricingTierNamedTuple provides a clear structure for tier pricing with a convenientlabelproperty for consistent formatting.
47-86: LGTM! Robust tier-aware pricing methods.The tier-aware methods correctly handle
Nonetier keys and missing tiers by falling back to default values. Theget_tier()method properly returnsNonewhen tiers don't exist or aren't found, and downstream methods handle this safely.
208-267: LGTM! Well-structured STANDARD plan with comprehensive tiers.The STANDARD plan provides 6 pricing tiers ranging from $25 to $400/month, with credits scaling appropriately (2,000 to 44,000). The plan structure, content, and tier definitions align well with the "power users" target audience.
352-356: LGTM! Comparison logic correctly uses tier-aware pricing.The comparison correctly uses
get_active_monthly_charge()for ordering plans, which defaults to base tier pricing when notier_keyis provided. The ENTERPRISE exception is appropriately maintained.
437-458: LGTM! Stripe line item generation correctly uses tier-aware pricing.The method properly retrieves tier-specific credits and monthly charges, and correctly passes
tier_keytoget_stripe_product_id()for tiered plans.
470-498: LGTM! Tier-aware Stripe product ID generation.The method correctly builds tier-specific product names (e.g., "Standard - 2,000 Credits for $25/month") and searches/creates Stripe products with the appropriate pricing. The logic properly handles both tiered and non-tiered plans.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
daras_ai_v2/billing.py (2)
282-288: Misleading docstring already flagged.The docstring claims this function returns a selected tier key, but it returns nothing. This issue was previously identified.
444-451: Defensive check for tier lookup already flagged.Lines 448-450 access
.monthly_chargedirectly afterget_tier(), which can returnNone. This AttributeError risk was previously identified.
🧹 Nitpick comments (3)
payments/plans.py (3)
82-85: Consider custom exception for better error handling.Static analysis suggests avoiding long messages directly in exception constructors. While the current implementation is functional, you could define a custom exception class if this pattern appears frequently.
Example refactor (if desired):
class NoTiersAvailableError(ValueError): def __init__(self, plan_key: str): super().__init__(f"No tiers available for plan {plan_key}") def get_default_tier_key(self) -> str: if self.tiers: return next(iter(self.tiers.keys())) raise NoTiersAvailableError(self.key)
382-416: Consider using_for unused loop variable.Static analysis notes that
tier_keyat line 399 is not used in the loop body. While this doesn't affect functionality, you could use_to signal the unused variable.Apply this diff if desired:
if plan.tiers: # Check each tier by matching product name - for tier_key, tier in plan.tiers.items(): + for _, tier in plan.tiers.items(): expected_name = f"{plan.title} - {tier.label}" expected_price = tier.monthly_charge * 100 # convert to centsOtherwise, the tier-aware product matching logic is well-structured and handles both tiered and non-tiered plans correctly.
425-429: Consider custom exception for consistency.Similar to line 85, static analysis suggests avoiding long messages in exception constructors. However, for this key lookup failure, the current implementation is clear and functional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
daras_ai_v2/billing.py(18 hunks)payments/plans.py(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
payments/plans.py (1)
payments/utils.py (1)
make_stripe_recurring_plan(4-31)
daras_ai_v2/billing.py (4)
payments/plans.py (10)
get_active_monthly_charge(76-80)get_active_credits(70-74)get_pricing_title(50-55)PricingPlan(88-538)from_sub(369-372)get_default_tier_key(82-85)get_pricing_caption(57-62)get_tier(64-68)get_stripe_line_item(437-458)supports_stripe(431-432)daras_ai_v2/user_date_widgets.py (1)
render_local_date_attrs(30-38)payments/models.py (1)
Subscription(37-399)workspaces/models.py (1)
allow_credit_topups(223-247)
🪛 Ruff (0.14.2)
payments/plans.py
85-85: Avoid specifying long messages outside the exception class
(TRY003)
399-399: Loop control variable tier_key not used within loop body
Rename unused tier_key to _tier_key
(B007)
429-429: Avoid specifying long messages outside the exception class
(TRY003)
439-439: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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). (2)
- GitHub Check: test (3.10.12, 1.8.3)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (21)
daras_ai_v2/billing.py (13)
82-143: LGTM! Tier-aware rendering in current plan display.The function correctly retrieves the tier key from the subscription and uses tier-aware pricing methods throughout. The next invoice date rendering and pricing display logic are well-structured.
152-192: LGTM! Plan filtering and rendering logic.The filtering correctly restricts STANDARD to personal workspaces (unless already subscribed), and the grid/full-width separation aligns with the new plan structure.
303-335: LGTM! Tier selection UI implementation.The tier selection logic correctly renders a selectbox for tiered plans when using Stripe, uses the current subscription's tier as default, and returns the selected tier key for downstream usage.
338-398: LGTM! Action button orchestration with tier support.The logic correctly handles tier changes within the same plan and delegates to appropriate subscription creation or change flows with tier context.
458-476: LGTM! Downgrade flow with tier-aware pricing.The downgrade confirmation dialog correctly displays tier-aware monthly charges using
get_active_monthly_chargeand passestier_keyto the subscription change function.
478-543: LGTM! Upgrade flow with tier-aware pricing.The upgrade confirmation dialog correctly retrieves and displays tier-aware monthly charges and credits. The tier_key is properly propagated through the subscription change flow. Error handling for payment failures was previously addressed.
545-576: LGTM! Subscription creation with tier support.The function correctly handles STANDARD as personal-only, passes tier_key through Stripe flows, and documents PayPal's lack of tier support.
578-582: LGTM! Simplified price formatting.The refactored signature accepting a numeric
monthly_chargeparameter supports tier-aware pricing nicely.
585-653: LGTM! Subscription change flow with tier support.The function correctly propagates tier_key through Stripe metadata and line items. The early return when both plan and tier are unchanged is a good optimization.
667-688: LGTM! Addon section gated by workspace allowance.The early return based on
workspace.allow_credit_topups()correctly implements the feature restriction for new non-enterprise users as per PR objectives.
809-854: LGTM! Stripe subscription button with tier support.The function correctly accepts tier_key, retrieves tier-aware pricing, and propagates the tier context through the subscription creation flow.
856-911: LGTM! Stripe subscription creation with tier support.The function properly accepts tier_key, includes it in Stripe metadata using the configured metadata field, and passes it through line item generation for both direct subscription and checkout flows.
1074-1157: LGTM! Auto-recharge section gated by workspace allowance.The early return based on
workspace.allow_credit_topups()correctly restricts auto-recharge access as per the PR objectives.payments/plans.py (8)
23-31: LGTM! Clean PricingTier definition.The NamedTuple structure is straightforward, and the label property provides a clear user-facing representation.
50-80: LGTM! Tier-aware pricing accessors.The accessor methods correctly handle tier lookups with appropriate fallbacks. The walrus operator usage in
get_active_creditsandget_active_monthly_chargeis clean and idiomatic.
208-267: LGTM! Comprehensive STANDARD plan definition.The plan is well-structured with six tiers offering a reasonable pricing progression. The description and footer accurately reflect the capabilities, and the tier naming convention is consistent.
172-206: LGTM! Improved STARTER plan messaging.The updated title "Free" and expanded feature list provide clearer value communication. The pricing_title and pricing_caption overrides work well for the free tier.
269-347: LGTM! BUSINESS and ENTERPRISE plan updates.The hierarchy is clear with BUSINESS referencing "Everything in Standard +", and ENTERPRISE's full_width flag correctly supports the UI rendering requirements.
349-362: LGTM! Plan-level comparison operators.The
__gt__operator correctly usesget_active_monthly_charge()without tier_key, which provides plan-level comparison. Tier-specific comparisons are handled separately in the billing logic where needed.
437-458: LGTM! Tier-aware Stripe line item generation.The function correctly retrieves tier-specific pricing and passes tier_key through to product ID generation. The ValueError for unsupported plans is appropriate, though static analysis flags the long message as a minor style issue.
470-498: LGTM! Tier-aware Stripe product ID resolution.The function correctly builds tier-specific product names that match the format expected by
get_by_stripe_product. The search and create logic properly handles both existing and new products.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
daras_ai_v2/billing.py (2)
444-450: Add defensive check for tier lookup in comparison logic.Lines 448-449 directly access
.monthly_chargeafterget_tier(), which returnsPricingTier | None. If the subscription contains an invalidselected_tier_keyorcurrent_tier_key(e.g., from database migration or stale data), this will raise anAttributeError.Apply this diff to add defensive handling:
if plan > current_plan or ( plan == current_plan and plan.tiers + and (current_tier := plan.get_tier(current_tier_key)) + and (selected_tier := plan.get_tier(selected_tier_key)) and ( - plan.get_tier(selected_tier_key).monthly_charge - > plan.get_tier(current_tier_key).monthly_charge + selected_tier.monthly_charge > current_tier.monthly_charge ) ):
282-288: Fix the misleading docstring.The docstring at line 283 claims the function "return selected tier key if plan has tiers," but the function doesn't return anything. The tier selection logic was moved to
_render_plan_pricing.Apply this diff to correct the docstring:
def _render_plan_details(plan: PricingPlan): - """Render plan details and return selected tier key if plan has tiers""" + """Render plan description and footer""" with gui.div():
🧹 Nitpick comments (2)
daras_ai_v2/billing.py (2)
303-336: Remove duplicate assignment.Line 333 unnecessarily assigns
current_planagain, which was already assigned on line 306. This is redundant and reduces code clarity.Apply this diff to remove the duplicate:
with pricing_div: with gui.tag("h3", className="my-0 d-inline me-2"): gui.html(plan.get_pricing_title(selected_tier_key)) with gui.tag("p", className="text-muted my-0"): gui.html(plan.get_pricing_caption(selected_tier_key)) - current_plan = PricingPlan.from_sub(workspace.subscription) return selected_tier_key
478-543: LGTM on upgrade flow, consider adding defensive check.The upgrade logic correctly handles tier-aware pricing and propagates
tier_keythrough the confirmation dialog and subscription creation. The tier-based pricing display at lines 505-507 is accurate.Note: Line 485 directly accesses
workspace.subscription.plan_tier_keywithout a None check. While the calling context ensures subscription exists, adding an assertion or defensive check would make this explicit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
daras_ai_v2/billing.py(18 hunks)
⏰ 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). (1)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (12)
daras_ai_v2/billing.py (12)
1-1: LGTM on the import.The
partialimport from functools is correctly used at lines 173-179 for the grid_layout function.
82-143: LGTM on tier-aware current plan rendering.The tier extraction from the subscription and tier-aware pricing display using
get_active_monthly_charge(tier_key)andget_active_credits(tier_key)are correctly implemented.
152-192: LGTM on plan filtering and rendering.The logic at lines 158-159 correctly restricts the STANDARD plan to personal workspaces while allowing existing STANDARD subscribers to see their current plan. The refactored rendering using
_render_plan_compactand_render_plan_full_widthhelpers improves maintainability.
195-280: LGTM on plan rendering helpers.The
_render_plan_full_widthand_render_plan_compactfunctions correctly delegate tier selection to_render_plan_pricingand propagateselected_tier_keythrough to the action buttons. The responsive layout handling is well-structured.
338-399: LGTM on action button logic.The tier comparison at line 354 correctly handles None cases through short-circuit evaluation, and the
selected_tier_keyis properly propagated through to subscription change and creation flows.
461-476: LGTM on downgrade flow.The downgrade logic correctly uses tier-aware pricing getters and passes
tier_keythrough tochange_subscription. The confirmation dialog appropriately displays the pricing comparison.
545-576: LGTM on subscription creation flow.The function correctly guards personal workflows for the STANDARD plan (lines 554-555) and appropriately propagates
selected_tier_keythrough Stripe flows while documenting that PayPal doesn't support tiers.
578-583: LGTM on price formatting.The updated signature accepting
monthly_charge: intcorrectly supports tier-aware pricing throughout the billing flows.
585-654: LGTM on subscription change logic.The tier-aware subscription changes are well-implemented:
- Defensive tier extraction at lines 592-594
- Early exit when plan and tier are unchanged (line 597)
- Proper metadata construction with tier info (lines 615-622)
- Correct tier propagation to Stripe line items (line 628)
667-673: LGTM on addon section gating.The early return based on
workspace.allow_credit_topups()correctly hides the credit purchase section for users without top-up permissions, aligning with the PR objectives to restrict addons for non-enterprise users.
809-911: LGTM on tier-aware Stripe integration.The Stripe subscription flows correctly integrate tier support:
- Tier-aware pricing display in confirmation dialogs (lines 822-823, 844, 848)
- Proper metadata construction with tier info (lines 882-884)
- Consistent tier propagation through both immediate and checkout flows
- Correct usage of
plan.get_stripe_line_item(tier_key)at line 884
1074-1081: LGTM on auto-recharge section gating.The early return guard based on
workspace.allow_credit_topups()correctly hides auto-recharge settings for users without top-up permissions, maintaining consistency with the addon section gating.
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on December 27. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
daras_ai_v2/billing.py(19 hunks)payments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.py(1 hunks)payments/models.py(5 hunks)payments/plans.py(10 hunks)payments/utils.py(1 hunks)payments/webhooks.py(4 hunks)usage_costs/migrations/0038_alter_modelpricing_model_name.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,js,ts,tsx,java,cs,cpp,c,go,rb,php}
📄 CodeRabbit inference engine (.cursor/rules/devs-rules.mdc)
Format code in reverse topological order: place the main() function at the top and dependencies below it
Files:
payments/webhooks.pypayments/utils.pypayments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.pyusage_costs/migrations/0038_alter_modelpricing_model_name.pypayments/models.pypayments/plans.pydaras_ai_v2/billing.py
🧬 Code graph analysis (3)
payments/webhooks.py (1)
payments/plans.py (3)
PricingPlan(81-506)get_by_key(394-398)get_by_stripe_product(351-385)
payments/models.py (1)
payments/plans.py (4)
PricingPlan(81-506)stripe_get_addon_product(522-531)PricingTier(23-29)from_sub(338-341)
payments/plans.py (1)
payments/utils.py (1)
make_stripe_recurring_plan(4-31)
🪛 Ruff (0.14.5)
payments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-50: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
26-26: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
usage_costs/migrations/0038_alter_modelpricing_model_name.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-298: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
payments/models.py
46-46: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
payments/plans.py
78-78: Avoid specifying long messages outside the exception class
(TRY003)
398-398: Avoid specifying long messages outside the exception class
(TRY003)
408-408: Avoid specifying long messages outside the exception class
(TRY003)
daras_ai_v2/billing.py
305-305: Local variable current_plan is assigned to but never used
Remove assignment to unused variable current_plan
(F841)
⏰ 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). (2)
- GitHub Check: test (3.10.12, 1.8.3)
- GitHub Check: Analyze (python)
🔇 Additional comments (15)
payments/utils.py (1)
20-20: LGTM! Precision increase supports tier-based pricing.The increase from 4 to 12 decimal places for
unit_amount_decimalenables more accurate per-credit pricing calculations, especially for tiered plans where credits and monthly charges vary. This aligns well with the tier-aware pricing introduced inpayments/plans.py.payments/plans.py (5)
23-29: LGTM! Clean tier data structure.The
PricingTierNamedTuple provides a clear, immutable representation of tier-specific pricing with a convenient label property.
49-73: LGTM! Tier-aware accessor methods are well-designed.The
get_active_credits()andget_active_monthly_charge()methods provide a consistent way to retrieve pricing information whether using tiered or default pricing. The fallback logic is clear and correct.
165-199: LGTM! STARTER plan correctly represents free tier.The updated STARTER plan clearly represents the free tier with appropriate pricing and feature descriptions. The pricing_title and pricing_caption effectively communicate the free offering.
228-235: LGTM! STANDARD plan tiers are well-structured.The tiered pricing for the STANDARD plan provides clear progression from 2,000 to 44,000 credits with corresponding pricing. The tier structure enables flexible scaling for power users.
351-385: LGTM! Tier-aware product matching is correctly implemented.The
get_by_stripe_product()method correctly handles both tiered and non-tiered plans by matching product names and prices. The logic properly falls back to legacy product name lookups and handles missing default_price gracefully.daras_ai_v2/billing.py (4)
82-87: LGTM! Tier-aware pricing correctly integrated.The current plan rendering now uses
get_tier()and tier-aware pricing accessors (get_active_monthly_charge,get_active_credits) to display accurate pricing information.
308-329: LGTM! Tier selection UI is well-implemented.The tier selection logic provides a clean selectbox for Stripe users when tiers are available, defaulting to the current charged_amount. The fallback to None for non-tiered plans is appropriate.
667-669: LGTM! Addon section properly gated.The
workspace.allow_credit_topups()check correctly hides the addon section for workspaces that aren't allowed to purchase topups.
1073-1075: LGTM! Auto-recharge section properly gated.The
workspace.allow_credit_topups()check correctly hides the auto-recharge section for workspaces that aren't allowed to use this feature.payments/models.py (2)
103-111: LGTM! Tier-aware full_clean correctly implemented.The
full_cleanmethod now derivesamountandcharged_amountfrom the plan when auto-recharge is enabled, properly supporting tier-aware pricing. The fallback logic ensures backward compatibility when these fields aren't set.
133-138: LGTM! get_tier method correctly constructs PricingTier.The
get_tier()method properly reconstructs aPricingTierfrom the storedamountandcharged_amountfields, converting charged_amount from cents to dollars. The None return when either field is missing is appropriate.payments/webhooks.py (2)
171-180: LGTM! Tier-aware plan lookup with proper fallback.The try/except logic correctly attempts to lookup the plan from metadata first, then falls back to product matching. The assertion with a descriptive message helps identify missing plan configurations.
280-284: LGTM! Tier-aware subscription data correctly persisted.The
set_workspace_subscriptionfunction now populatesamountandcharged_amountwith sensible fallbacks (plan defaults when not provided), and callsfull_clean()to ensure auto-recharge parameters are properly initialized.usage_costs/migrations/0038_alter_modelpricing_model_name.py (1)
26-26: Fix Unicode apostrophe in help_text.Line 26 contains a Unicode right single quotation mark (') in "Learn More" instead of a standard ASCII apostrophe ('). This could cause encoding issues in some environments.
Apply this diff:
- help_text="The name of the model. Only used for Display purposes.<a href='https://stripe.com/docs/currencies'>Learn More</a>", + help_text="The name of the model. Only used for Display purposes.<a href='https://stripe.com/docs/currencies'>Learn More</a>",Likely an incorrect or invalid review comment.
| name="charged_amount", | ||
| field=models.PositiveIntegerField( | ||
| default=0, | ||
| help_text="The charged dollar amount in the currency’s smallest unit.<br>E.g. for 10 USD, this would be of 1000 (that is, 1000 cents).<br><a href='https://stripe.com/docs/currencies'>Learn More</a>", |
There was a problem hiding this comment.
Fix Unicode apostrophe in help_text.
Line 26 contains a Unicode right single quotation mark (') in the link instead of a standard ASCII apostrophe ('). This could cause encoding issues.
Apply this diff:
- help_text="The charged dollar amount in the currency's smallest unit.<br>E.g. for 10 USD, this would be of 1000 (that is, 1000 cents).<br><a href='https://stripe.com/docs/currencies'>Learn More</a>",
+ help_text="The charged dollar amount in the currency's smallest unit.<br>E.g. for 10 USD, this would be of 1000 (that is, 1000 cents).<br><a href='https://stripe.com/docs/currencies'>Learn More</a>",Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.5)
26-26: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
🤖 Prompt for AI Agents
In
payments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.py
around line 26, the help_text contains a Unicode right single quotation mark in
the "<a href='https://stripe.com/docs/currencies'>Learn More</a>" part; replace
that Unicode apostrophe with a standard ASCII single quote (') so the href uses
plain ASCII quotes, then save the file with UTF-8 encoding (no other changes).
| amount = models.IntegerField( | ||
| help_text="The amount (Gooey credits) added/deducted in this transaction.<br>" | ||
| "Positive for credits added, negative for credits deducted.", | ||
| default=0, | ||
| ) | ||
| charged_amount = models.PositiveIntegerField( | ||
| help_text="The charged dollar amount in the currency’s smallest unit.<br>" | ||
| "E.g. for 10 USD, this would be of 1000 (that is, 1000 cents).<br>" | ||
| "<a href='https://stripe.com/docs/currencies'>Learn More</a>", | ||
| default=0, | ||
| ) |
There was a problem hiding this comment.
Fix Unicode apostrophe in help_text.
Line 48 contains a Unicode right single quotation mark (') in the link instead of a standard ASCII apostrophe ('). This could cause encoding issues.
Apply this diff:
charged_amount = models.PositiveIntegerField(
help_text="The charged dollar amount in the currency's smallest unit.<br>"
"E.g. for 10 USD, this would be of 1000 (that is, 1000 cents).<br>"
- "<a href='https://stripe.com/docs/currencies'>Learn More</a>",
+ "<a href='https://stripe.com/docs/currencies'>Learn More</a>",
default=0,
)Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.5)
46-46: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
🤖 Prompt for AI Agents
In payments/models.py around lines 40 to 50, the help_text string for
charged_amount contains a Unicode right single quotation mark in “currency’s”;
replace that character with a standard ASCII apostrophe so the text reads
"currency's" (ensure the string literal is updated accordingly and saved in
UTF-8 without changing other text).
devxpy
force-pushed
the
subscription-plan-standard
branch
from
a8918af to
510c49c
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
payments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.py (1)
26-26: Unicode apostrophe in help_text (static analysis flag).Ruff flagged an ambiguous Unicode character in the
help_text. The'characters inhref='...'may be Unicode right single quotation marks instead of ASCII single quotes. While this likely works fine, it could cause encoding issues in some contexts.If you want to fix this, ensure the quotes are standard ASCII:
- help_text="The charged dollar amount in the currency's smallest unit.<br>E.g. for 10 USD, this would be of 1000 (that is, 1000 cents).<br><a href='https://stripe.com/docs/currencies'>Learn More</a>", + help_text="The charged dollar amount in the currency's smallest unit.<br>E.g. for 10 USD, this would be of 1000 (that is, 1000 cents).<br><a href='https://stripe.com/docs/currencies'>Learn More</a>",daras_ai_v2/billing.py (2)
305-305: Remove unused variablecurrent_plan.The
current_planvariable is assigned but never used in_render_plan_pricing. This was flagged by static analysis.Apply this diff:
def _render_plan_pricing( plan: PricingPlan, payment_provider: PaymentProvider | None, workspace: "Workspace" ) -> PricingTier | None: - current_plan = PricingPlan.from_sub(workspace.subscription) pricing_div = gui.div()
441-446: Add None check forselected_tierbefore accessing.monthly_charge.Line 445 accesses
selected_tier.monthly_chargewithout verifyingselected_tieris not None. Whilecurrent_tieris checked on line 444,selected_tiercan still be None if passed from a call site like line 385. This will raiseAttributeError.Apply this diff:
if plan > current_plan or ( plan == current_plan and plan.tiers and current_tier + and selected_tier and selected_tier.monthly_charge > current_tier.monthly_charge ):payments/webhooks.py (1)
171-180: Plan lookup fallback still uses assertion that could crash webhook.Lines 179-180 assert that
plan is not Nonewhen falling back to product lookup. This will crash the webhook handler on legacy subscriptions or products without matching plan definitions, causing Stripe to retry indefinitely. The past review suggested graceful handling with logging and early return.Apply this diff for graceful handling:
try: plan = PricingPlan.get_by_key( stripe_sub.metadata[settings.STRIPE_USER_SUBSCRIPTION_METADATA_FIELD] ) except KeyError: product = stripe.Product.retrieve( stripe_sub.plan.product, expand=["default_price"] ) plan = PricingPlan.get_by_stripe_product(product) - assert plan is not None, f"Plan for product {product.id} not found" + if plan is None: + logger.warning(f"Plan not found for Stripe product {product.id}") + return # Acknowledge webhook to prevent retries
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
daras_ai_v2/billing.py(19 hunks)payments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.py(1 hunks)payments/models.py(5 hunks)payments/plans.py(10 hunks)payments/utils.py(1 hunks)payments/webhooks.py(4 hunks)usage_costs/migrations/0038_alter_modelpricing_model_name.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- payments/utils.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,js,ts,tsx,java,cs,cpp,c,go,rb,php}
📄 CodeRabbit inference engine (.cursor/rules/devs-rules.mdc)
Format code in reverse topological order: place the main() function at the top and dependencies below it
Files:
payments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.pyusage_costs/migrations/0038_alter_modelpricing_model_name.pypayments/webhooks.pypayments/plans.pypayments/models.pydaras_ai_v2/billing.py
🧬 Code graph analysis (3)
payments/plans.py (2)
payments/utils.py (1)
make_stripe_recurring_plan(4-31)daras_ai_v2/paypal.py (2)
Product(237-242)create(82-92)
payments/models.py (1)
payments/plans.py (4)
PricingPlan(81-506)stripe_get_addon_product(522-531)PricingTier(23-29)from_sub(338-341)
daras_ai_v2/billing.py (2)
payments/plans.py (10)
PricingPlan(81-506)PricingTier(23-29)get_active_monthly_charge(69-73)get_active_credits(63-67)get_pricing_title(49-54)label(28-29)from_sub(338-341)get_pricing_caption(56-61)get_stripe_line_item(406-427)supports_stripe(400-401)payments/models.py (2)
get_tier(133-138)Subscription(37-403)
🪛 Ruff (0.14.5)
payments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-50: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
26-26: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
usage_costs/migrations/0038_alter_modelpricing_model_name.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-298: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
payments/plans.py
78-78: Avoid specifying long messages outside the exception class
(TRY003)
398-398: Avoid specifying long messages outside the exception class
(TRY003)
408-408: Avoid specifying long messages outside the exception class
(TRY003)
payments/models.py
46-46: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
daras_ai_v2/billing.py
305-305: Local variable current_plan is assigned to but never used
Remove assignment to unused variable current_plan
(F841)
⏰ 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). (2)
- GitHub Check: Analyze (python)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (22)
usage_costs/migrations/0038_alter_modelpricing_model_name.py (1)
8-10: Ignore static analysis hint (false positive).The Ruff warning about
ClassVarannotations ondependenciesandoperationsis a false positive. This is auto-generated Django migration code following Django's standard conventions. These attributes should not be modified with type annotations as they follow Django's expected migration structure.Also applies to: 12-298
daras_ai_v2/billing.py (6)
1-1: Import moved to top of file.The
functools.partialimport placement looks good for the new usage inrender_all_plans.
82-141: Good tier-aware pricing integration inrender_current_plan.The tier retrieval and tier-aware pricing methods (
get_active_monthly_charge,get_active_credits,get_pricing_title) are properly used, with graceful fallbacks when tier is None.
151-191: Plan rendering refactored for tier support and layout.The STANDARD plan filtering for non-personal workspaces and the grid/full-width plan separation are well-structured. The
partialusage for grid layout is clean.
664-669: Good gating of add-on and auto-recharge sections.The
workspace.allow_credit_topups()check properly hides these sections for workspaces that shouldn't have access to credit top-ups, aligning with the PR objective to disable auto-recharge/addons for non-enterprise users.Also applies to: 1069-1075
851-906: Tier-aware Stripe subscription creation looks correct.The
tierparameter is properly threaded through toplan.get_stripe_line_item(tier)for line item generation.
582-650: Tier-aware subscription change looks correct.The early exit check (lines 593-598) properly compares both plan and tier. The Stripe modification correctly uses tier-aware line items. The metadata stores only the plan key, which is appropriate since tier details are embedded in the line item pricing.
payments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.py (1)
12-49: Migration operations look correct.The new
amountandcharged_amountfields are properly added with sensible defaults. Theauto_recharge_enableddefault change toFalsealigns with the PR objective to disable auto-recharge for new users. The plan choices include the new STANDARD plan.payments/webhooks.py (2)
188-198: Amount and charged_amount calculation looks correct.The calculation properly uses
Decimalfor precision when computingcharged_amountfrom Stripe'samount_decimal. The values are correctly passed toset_workspace_subscription.
260-293:set_workspace_subscriptioncorrectly handles tier data.The new
amountandcharged_amountparameters with fallbacks to plan defaults (lines 279-280) are well-designed. Thefull_clean()call without explicit parameters aligns with the model's updated signature.payments/models.py (5)
40-50: New subscription fields for tier data look correct.The
amountandcharged_amountfields are properly defined with descriptive help_text and sensible defaults. These fields support the tier-aware pricing system.
45-48: Unicode apostrophe in help_text.Same Unicode character issue as in the migration file - the
'in the href may be a Unicode right single quotation mark.
103-111: Simplifiedfull_cleanwith tier-aware defaults.The refactored
full_cleancorrectly derivesamountandcharged_amountfrom subscription fields or falls back to plan defaults. This aligns with the tier-aware pricing system.
133-138:get_tier()method correctly derives tier from subscription data.The method properly guards against missing data and converts
charged_amount(in cents) tomonthly_charge(in dollars) when constructing thePricingTier.
61-61: Defaultauto_recharge_enabled=Falsealigns with PR objectives.This change disables auto-recharge by default for new subscriptions, matching the PR goal to disable auto-recharge/addons for new non-enterprise users.
payments/plans.py (7)
23-30: CleanPricingTierimplementation.The
PricingTiernamedtuple withcredits,monthly_charge, and a formattedlabelproperty is well-designed for tier-aware pricing.
49-78: Tier-aware accessor methods are well-designed.The accessor methods (
get_pricing_title,get_pricing_caption,get_active_credits,get_active_monthly_charge) properly handle both tier-specific and default pricing scenarios. Theget_default_tiermethod appropriately raises an error for plans without tiers.
201-236: New STANDARD plan with tiered pricing looks correct.The plan has 6 well-structured tiers ranging from 2,000 credits/$25 to 44,000 credits/$400. The long description accurately reflects the tier range ("Up to 44,000 Cr / month").
350-385: Comprehensive Stripe product lookup with tier support.The
get_by_stripe_productmethod correctly handles:
- Legacy products via
REVERSE_STRIPE_PRODUCT_NAMES- Tiered plans by matching product name and price to tier labels
- Non-tiered plans by matching product name and default price
Returning
Nonefor unmatched products allows callers to handle gracefully.
406-427: Tier-aware Stripe line item generation is correct.The method properly uses tier-aware pricing methods and passes the tier through to
get_stripe_product_idfor product lookup/creation.
439-466: Tier-aware Stripe product ID resolution is well-implemented.The method:
- Builds a tier-specific product name (e.g., "Standard - 2,000 Credits for $25/month")
- Matches existing products by both name and price
- Creates new products with correct tier-specific pricing if not found
This design allows multiple Stripe products per plan, one for each tier.
321-325: Plan comparison logic is correct.Using
get_active_monthly_charge()without a tier argument compares plans by their base monthly charge. This is appropriate since tier selection affects pricing within a plan, not the plan ordering.
| ("protogen_2_2", "Protogen V2.2 (darkstorm2150)"), | ||
| ("epicdream", "epiCDream [Deprecated] (epinikion)"), | ||
| ("flux_1_dev", "FLUX.1 [dev]"), | ||
| ("dream_shaper", "DreamShaper (Lykon)"), | ||
| ("dreamlike_2", "Dreamlike Photoreal 2.0 (dreamlike.art)"), | ||
| ("sd_2", "Stable Diffusion v2.1 (stability.ai)"), | ||
| ("sd_1_5", "Stable Diffusion v1.5 (RunwayML)"), | ||
| ("dall_e", "DALL·E 2 (OpenAI)"), | ||
| ("dall_e_3", "DALL·E 3 (OpenAI)"), | ||
| ("gpt_image_1", "GPT Image 1 (OpenAI)"), | ||
| ("nano_banana", "Nano Banana (Google)"), | ||
| ("openjourney_2", "Open Journey v2 beta [Deprecated] (PromptHero)"), | ||
| ("openjourney", "Open Journey [Deprecated] (PromptHero)"), | ||
| ("analog_diffusion", "Analog Diffusion [Deprecated] (wavymulder)"), | ||
| ("protogen_5_3", "Protogen v5.3 [Deprecated] (darkstorm2150)"), | ||
| ("jack_qiao", "Stable Diffusion v1.4 [Deprecated] (Jack Qiao)"), | ||
| ( | ||
| "rodent_diffusion_1_5", | ||
| "Rodent Diffusion 1.5 [Deprecated] (NerdyRodent)", | ||
| ), | ||
| ("deepfloyd_if", "DeepFloyd IF [Deprecated] (stability.ai)"), | ||
| ("flux_pro_kontext", "FLUX.1 Pro Kontext (fal.ai)"), | ||
| ("dream_shaper", "DreamShaper (Lykon)"), | ||
| ("dreamlike_2", "Dreamlike Photoreal 2.0 (dreamlike.art)"), | ||
| ("sd_2", "Stable Diffusion v2.1 (stability.ai)"), | ||
| ("sd_1_5", "Stable Diffusion v1.5 (RunwayML)"), | ||
| ("dall_e", "Dall-E (OpenAI)"), | ||
| ("gpt_image_1", "GPT Image 1 (OpenAI)"), | ||
| ("nano_banana", "Nano Banana (Google)"), | ||
| ("instruct_pix2pix", "✨ InstructPix2Pix (Tim Brooks)"), | ||
| ("openjourney_2", "Open Journey v2 beta [Deprecated] (PromptHero)"), | ||
| ("openjourney", "Open Journey [Deprecated] (PromptHero)"), | ||
| ("analog_diffusion", "Analog Diffusion [Deprecated] (wavymulder)"), | ||
| ("protogen_5_3", "Protogen v5.3 [Deprecated] (darkstorm2150)"), | ||
| ("jack_qiao", "Stable Diffusion v1.4 [Deprecated] (Jack Qiao)"), | ||
| ( | ||
| "rodent_diffusion_1_5", | ||
| "Rodent Diffusion 1.5 [Deprecated] (NerdyRodent)", | ||
| ), | ||
| ("sd_2", "Stable Diffusion v2.1 (stability.ai)"), | ||
| ("runway_ml", "Stable Diffusion v1.5 (RunwayML)"), | ||
| ("dall_e", "Dall-E (OpenAI)"), | ||
| ("jack_qiao", "Stable Diffusion v1.4 [Deprecated] (Jack Qiao)"), |
There was a problem hiding this comment.
Remove duplicate model choices.
The choices list contains numerous duplicate keys starting around line 245. For example:
dream_shaperappears at both lines 248 and 267sd_2appears three times (lines 250, 269, 284)dall_eappears three times with inconsistent display names (line 252: "DALL·E 2 (OpenAI)" vs lines 271, 286: "Dall-E (OpenAI)")jack_qiaoappears three times (lines 260, 279, 287)- Many others:
dreamlike_2,sd_1_5,gpt_image_1,nano_banana,openjourney_2,openjourney,analog_diffusion,protogen_5_3,rodent_diffusion_1_5
Django will only use the first occurrence of each key, rendering the duplicates ineffective and creating maintenance confusion.
Apply this diff to remove the duplicate entries:
("epicdream", "epiCDream [Deprecated] (epinikion)"),
("flux_1_dev", "FLUX.1 [dev]"),
- ("dream_shaper", "DreamShaper (Lykon)"),
- ("dreamlike_2", "Dreamlike Photoreal 2.0 (dreamlike.art)"),
- ("sd_2", "Stable Diffusion v2.1 (stability.ai)"),
- ("sd_1_5", "Stable Diffusion v1.5 (RunwayML)"),
- ("dall_e", "DALL·E 2 (OpenAI)"),
("dall_e_3", "DALL·E 3 (OpenAI)"),
- ("gpt_image_1", "GPT Image 1 (OpenAI)"),
- ("nano_banana", "Nano Banana (Google)"),
- ("openjourney_2", "Open Journey v2 beta [Deprecated] (PromptHero)"),
- ("openjourney", "Open Journey [Deprecated] (PromptHero)"),
- ("analog_diffusion", "Analog Diffusion [Deprecated] (wavymulder)"),
- ("protogen_5_3", "Protogen v5.3 [Deprecated] (darkstorm2150)"),
- ("jack_qiao", "Stable Diffusion v1.4 [Deprecated] (Jack Qiao)"),
- (
- "rodent_diffusion_1_5",
- "Rodent Diffusion 1.5 [Deprecated] (NerdyRodent)",
- ),
("deepfloyd_if", "DeepFloyd IF [Deprecated] (stability.ai)"),
("flux_pro_kontext", "FLUX.1 Pro Kontext (fal.ai)"),
("dream_shaper", "DreamShaper (Lykon)"),
("dreamlike_2", "Dreamlike Photoreal 2.0 (dreamlike.art)"),
("sd_2", "Stable Diffusion v2.1 (stability.ai)"),
("sd_1_5", "Stable Diffusion v1.5 (RunwayML)"),
- ("dall_e", "Dall-E (OpenAI)"),
- ("gpt_image_1", "GPT Image 1 (OpenAI)"),
- ("nano_banana", "Nano Banana (Google)"),
+ ("dall_e", "DALL·E 2 (OpenAI)"),
+ ("gpt_image_1", "GPT Image 1 (OpenAI)"),
+ ("nano_banana", "Nano Banana (Google)"),
("instruct_pix2pix", "✨ InstructPix2Pix (Tim Brooks)"),
("openjourney_2", "Open Journey v2 beta [Deprecated] (PromptHero)"),
("openjourney", "Open Journey [Deprecated] (PromptHero)"),
("analog_diffusion", "Analog Diffusion [Deprecated] (wavymulder)"),
("protogen_5_3", "Protogen v5.3 [Deprecated] (darkstorm2150)"),
("jack_qiao", "Stable Diffusion v1.4 [Deprecated] (Jack Qiao)"),
(
"rodent_diffusion_1_5",
"Rodent Diffusion 1.5 [Deprecated] (NerdyRodent)",
),
- ("sd_2", "Stable Diffusion v2.1 (stability.ai)"),
("runway_ml", "Stable Diffusion v1.5 (RunwayML)"),
- ("dall_e", "Dall-E (OpenAI)"),
- ("jack_qiao", "Stable Diffusion v1.4 [Deprecated] (Jack Qiao)"),
("wav2lip", "LipSync (wav2lip)"),
("sadtalker", "LipSync (sadtalker)"),
("twilio_voice", "Twilio Voice"),
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
payments/webhooks.py (1)
170-185: Fallback Stripe plan lookup can still crash webhook on unknown products.In
handle_subscription_updated, the fallback path:product = stripe.Product.retrieve(stripe_sub.plan.product, expand=["default_price"]) plan = PricingPlan.get_by_stripe_product(product) assert plan is not None, f"Plan for product {product.id} not found"will raise and 5xx the webhook if Stripe contains a legacy/misconfigured product that doesn’t map to any
PricingPlan, causing Stripe to retry indefinitely. Prefer logging and returning early instead of asserting:- plan = PricingPlan.get_by_stripe_product(product) - assert plan is not None, f"Plan for product {product.id} not found" + plan = PricingPlan.get_by_stripe_product(product) + if plan is None: + logger.warning(f"Plan not found for Stripe product {product.id}") + return # Acknowledge webhook; avoid retry loopdaras_ai_v2/billing.py (2)
333-472: Tier-aware upgrade/downgrade flows are consistent; main concern is broad Stripe error fallback.Positives:
render_change_subscription_buttonand_render_upgrade_subscription_buttoncorrectly comparePricingPlan+PricingTierviaget_active_monthly_chargeand passselected_tierthrough tochange_subscription.fmt_priceand the downgrade/upgrade dialogs now show the current plan’s monthly charge vs the new plan/tier, which matches the earlier feedback.change_subscriptionusesnew_plan.get_stripe_line_item(new_tier)and writesmetadata={settings.STRIPE_USER_SUBSCRIPTION_METADATA_FIELD: new_plan.key}, aligning webhook plan resolution withStripeWebhookHandler.handle_subscription_updated.However, the upgrade error handling still cancels and recreates the subscription on any
stripe.InvalidRequestError:except (stripe.CardError, stripe.InvalidRequestError) as e: if isinstance(e, stripe.InvalidRequestError): sentry_sdk.capture_exception(e) logger.warning(e) # only handle error if it's related to mandates workspace.subscription.cancel() stripe_subscription_create(workspace, plan, tier=selected_tier)This can aggressively cancel for configuration/temporary API errors that aren’t mandate-related. Consider the earlier refinement: handle
stripe.CardErroras the “payment failed, recreate” path, and narrowstripe.InvalidRequestErrorhandling to known mandate/payment-method cases (e.g., by checking error codes/message), re-raising others instead of canceling the subscription.Also applies to: 474-539, 574-599, 611-627
281-287: Fix misleading docstring for_render_plan_details(does not return a tier key).The docstring still says:
"""Render plan details and return selected tier key if plan has tiers"""but
_render_plan_detailsonly writeslong_descriptionandfooterand returnsNone. Update this to reflect actual behavior, e.g.:-def _render_plan_details(plan: PricingPlan): - """Render plan details and return selected tier key if plan has tiers""" +def _render_plan_details(plan: PricingPlan): + """Render plan description and footer"""
🧹 Nitpick comments (3)
payments/webhooks.py (1)
192-202: Be explicit about None vs 0 when defaulting amount/charged_amount.
amountandcharged_amountare derived from Stripe inhandle_subscription_updatedand defaulted inset_workspace_subscriptionas:new_sub.amount = amount or plan.credits new_sub.charged_amount = charged_amount or (plan.monthly_charge * 100)Using
ormakes it impossible to intentionally persist0for these fields in the future (it will silently fall back to plan defaults). If you ever introduce free/zero-amount tiers that still need explicit(amount=0, charged_amount=0), consider switching to explicit None checks:- new_sub.amount = amount or plan.credits - new_sub.charged_amount = charged_amount or (plan.monthly_charge * 100) + new_sub.amount = plan.credits if amount is None else amount + new_sub.charged_amount = ( + plan.monthly_charge * 100 if charged_amount is None else charged_amount + )Also applies to: 282-288
daras_ai_v2/billing.py (2)
151-192: STANDARD plan filtering logic: works now, butremoveis brittle if STANDARD is ever deprecated.
render_all_plansbuildsall_plans = [plan for plan in PricingPlan if not plan.deprecated]and then:if not workspace.is_personal and current_plan != PricingPlan.STANDARD: all_plans.remove(PricingPlan.STANDARD)This correctly hides STANDARD for non-personal workspaces unless they’re already on STANDARD (matching the “personal-only” intent), but
removewill raiseValueErrorif STANDARD is ever markeddeprecated=Trueor removed from the enum. To future-proof this, consider guarding:- if not workspace.is_personal and current_plan != PricingPlan.STANDARD: - all_plans.remove(PricingPlan.STANDARD) + if ( + not workspace.is_personal + and current_plan != PricingPlan.STANDARD + and PricingPlan.STANDARD in all_plans + ): + all_plans.remove(PricingPlan.STANDARD)
194-330: Tier selection UI wiring looks good; consider defending against missingcharged_amounton legacy subs.The new
_render_plan_full_width/_render_plan_compact+_render_plan_pricingflow cleanly propagates aPricingTier:
- Stripe +
plan.tiers→ selectbox keyed bytier.monthly_charge- Default value from
round(workspace.subscription.charged_amount / 100)- Returned
selected_tierused for all downstream pricing text and actions.If
subscription.charged_amountis guaranteed non-null (e.g., via migrations/defaults), this is fine. If older rows may still havecharged_amountasNULL,round(workspace.subscription.charged_amount / 100)will raise. A small defensive tweak would make this robust to partially-migrated data:- value=( - workspace.subscription - and round(workspace.subscription.charged_amount / 100) - ), + value=( + workspace.subscription + and workspace.subscription.charged_amount + and round(workspace.subscription.charged_amount / 100) + ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
daras_ai_v2/billing.py(19 hunks)payments/webhooks.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,js,ts,tsx,java,cs,cpp,c,go,rb,php}
📄 CodeRabbit inference engine (.cursor/rules/devs-rules.mdc)
Format code in reverse topological order: place the main() function at the top and dependencies below it
Files:
payments/webhooks.pydaras_ai_v2/billing.py
🧬 Code graph analysis (2)
payments/webhooks.py (2)
payments/plans.py (3)
PricingPlan(81-506)get_by_key(394-398)get_by_stripe_product(351-385)payments/models.py (1)
full_clean(103-111)
daras_ai_v2/billing.py (3)
payments/plans.py (8)
PricingPlan(81-506)PricingTier(23-29)get_active_monthly_charge(69-73)get_active_credits(63-67)get_pricing_title(49-54)label(28-29)from_sub(338-341)get_pricing_caption(56-61)payments/models.py (2)
get_tier(133-138)Subscription(37-403)workspaces/models.py (1)
allow_credit_topups(223-247)
⏰ 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). (1)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (4)
payments/webhooks.py (1)
95-140: Stripe invoice handler: metadata-based plan tagging looks consistent.Using
invoice.subscription_details.metadata[settings.STRIPE_USER_SUBSCRIPTION_METADATA_FIELD]to look up thePricingPlanand taggingadd_balance_for_paymentviakwargs["plan"]aligns with howchange_subscriptionandstripe_subscription_createwrite subscription metadata, and Sentry capture onKeyErrorgives observability without breaking the webhook. The amount/charged_amount extraction frominvoice.lines.data[0]also matches the new Subscription fields’ semantics.daras_ai_v2/billing.py (3)
82-142: Tier-aware current plan rendering is consistent with new Subscription fields.
render_current_plannow derivestier = workspace.subscription.get_tier()and usesplan.get_active_monthly_charge(tier)/plan.get_active_credits(tier)along withplan.get_pricing_title(tier), which matches thePricingTiersemantics inpayments.plansandSubscription.get_tier. The localized “Next invoice on …” badge viarender_local_date_attrsis also a clean UX improvement.
541-571: Stripe subscription creation/change now propagates tier correctly and matches webhook expectations.
render_stripe_subscription_buttonand_render_create_subscription_buttonpass aPricingTier | Nonedown astier.stripe_subscription_createbuildsline_items = [plan.get_stripe_line_item(tier)]and setsmetadata = {settings.STRIPE_USER_SUBSCRIPTION_METADATA_FIELD: plan.key}, also mirroring that metadata intosubscription_data.change_subscriptionsimilarly usesnew_plan.get_stripe_line_item(new_tier)and updates subscription metadata with the plan key.This lines up with:
PricingPlan.get_stripe_line_item(tier)semantics for tiered pricing.StripeWebhookHandler.handle_subscription_updated, which first reads the plan key from subscription metadata and then usesamount/charged_amountto rebuild the tier.The Standard-plan-specific workspace popup gating also looks coherent: Standard skips the “create workspace” popup for personal workspaces, while other paid plans still encourage workspace creation.
Also applies to: 805-848, 850-905, 581-599, 615-627
663-685: Addon & auto-recharge gating viaallow_credit_topups()matches PR intent.Both
render_addon_sectionandrender_auto_recharge_sectionnow early-return whennot workspace.allow_credit_topups(), so:
- New non-enterprise workspaces without prior ADDON/AUTO_RECHARGE history won’t see purchase UI or auto-recharge controls.
- Enterprise (and existing topup-using) workspaces still see these sections.
This is a clean, model-level gate that aligns with the “disable auto recharge / addons for new non-enterprise users” objective.
Also applies to: 1068-1075
|
@devxpy i added a couple of fixes. one to render the plan change correctly when downloading. other to not 500-error in a webhook call (only log it) so stripe doesnt keep retrying. this looks good for merge to me. will have to sync this with the pricing info page (webflow) for deployment |
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
payments/webhooks.py (1)
35-37: Replace plan assertions with graceful handling to avoid webhook 5xx and retries.
asserton plan lookup in both PayPal (PricingPlan.get_by_paypal_plan_id) and Stripe (fallback viaPricingPlan.get_by_stripe_product) will still raise and turn these webhooks into 5xx responses whenever a plan cannot be resolved (legacy products, deleted plans, misconfig, etc.). That causes Stripe/PayPal to keep retrying and can spam logs without a way to fix from the dashboard.Instead of asserting, log and return early so the webhook is acknowledged:
- plan = PricingPlan.get_by_paypal_plan_id(pp_sub.plan_id) - assert plan, f"Plan {pp_sub.plan_id} not found" + plan = PricingPlan.get_by_paypal_plan_id(pp_sub.plan_id) + if plan is None: + logger.warning( + "PayPal subscription plan not found; skipping webhook", + plan_id=pp_sub.plan_id, + subscription_id=pp_sub.id, + ) + return- product = stripe.Product.retrieve( - stripe_sub.plan.product, expand=["default_price"] - ) - plan = PricingPlan.get_by_stripe_product(product) - assert plan is not None, f"Plan for product {product.id} not found" + product = stripe.Product.retrieve( + stripe_sub.plan.product, expand=["default_price"] + ) + plan = PricingPlan.get_by_stripe_product(product) + if plan is None: + logger.warning( + "Stripe subscription plan not found from product; skipping webhook", + product_id=product.id, + subscription_id=stripe_sub.id, + ) + # optionally: sentry_sdk.capture_message(...) + returnThis keeps your DB consistent with known plans while preventing endless retries on subscriptions you can’t map anymore.
#!/bin/bash # Quick check: see all call sites of these handlers to confirm # that a simple early return will still result in a 2xx webhook response. rg -n "handle_subscription_updated|handle_sale_completed" --type=py -C3Also applies to: 64-66, 175-185
🧹 Nitpick comments (1)
payments/webhooks.py (1)
100-107: Stripe invoice plan resolution change looks correct; consider adding contextual logging.Catching
KeyErrorfromPricingPlan.get_by_key(...)and reporting it to Sentry avoids 500s when metadata is missing or invalid, which aligns with the stated goal. You might also want alogger.warningincludinginvoice.idand the offending key so local logs are diagnosable even without Sentry, but behavior-wise this is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app_users/tasks.py(0 hunks)payments/webhooks.py(5 hunks)
💤 Files with no reviewable changes (1)
- app_users/tasks.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,js,ts,tsx,java,cs,cpp,c,go,rb,php}
📄 CodeRabbit inference engine (.cursor/rules/devs-rules.mdc)
Format code in reverse topological order: place the main() function at the top and dependencies below it
Files:
payments/webhooks.py
🧬 Code graph analysis (1)
payments/webhooks.py (1)
payments/plans.py (3)
PricingPlan(81-506)get_by_key(394-398)get_by_stripe_product(351-385)
⏰ 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). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (1)
payments/webhooks.py (1)
270-271: Based on my thorough verification of the codebase, I can now provide my rewritten review comment:
The validation concern is incorrect; however, semantic clarity around
amount/charged_amountdefaults could be improved.
set_workspace_subscriptiondefaultsamountandcharged_amountto0, which actually passes validation without issues:
amountisIntegerField(default=0)— allows any integer, including 0charged_amountisPositiveIntegerField(default=0)— allows 0 and positive integers- The
full_clean()validation only runs ifauto_recharge_enabled=True, and if so,ensure_default_auto_recharge_params()returns early when amounts ≤ 0 (no error)However, the semantics warrant clarification:
- PayPal flows (activation, updates, cancellation) never pass real amounts — always use 0
- Stripe flows pass explicit
amountandcharged_amountexcept during cancellation- All cancellation flows intentionally use 0 as a sentinel for "no active paid subscription"
The current code works correctly because PayPal uses plan-based pricing (no custom amounts) and Stripe provides explicit values. But the intent — that 0 represents "not applicable" rather than a genuine zero charge — could be documented more clearly in the function signature or via comments.
No validation will fail; this is a clarity and maintainability concern, not a bug.
| amount = int(stripe_sub.quantity) | ||
| charged_amount = round(Decimal(stripe_sub.plan.amount_decimal) * amount) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Stripe Python API plan.amount_decimal smallest currency unit cents documentation
💡 Result:
Short answer
- amount (or unit_amount) is an integer in the currency's smallest unit (e.g., cents for USD). [1][2]
- amount_decimal (or unit_amount_decimal / plan.amount_decimal) is the same value represented as a decimal string (can express fractional cents up to ~12 decimal places) and will be returned as a string in client libraries (including Python). Use amount for whole-smallest-unit integers and amount_decimal when you need the precise decimal representation. [1][3]
Sources
- Stripe API — example plan/price showing amount and amount_decimal. [1]
- Stripe API — amounts are specified in the smallest currency unit (e.g., cents). [2]
- Plan object docs / client-library descriptions noting amount_decimal is a decimal string (≤12 decimal places). [3]
🏁 Script executed:
#!/bin/bash
# Search for charged_amount usage and definition in the codebase
rg -n "charged_amount" --type py -B 3 -A 3Repository: GooeyAI/gooey-server
Length of output: 14138
🏁 Script executed:
#!/bin/bash
# Search for handle_invoice_paid function mentioned in the review
rg -n "handle_invoice_paid" --type py -B 2 -A 10Repository: GooeyAI/gooey-server
Length of output: 3779
🏁 Script executed:
#!/bin/bash
# Examine the payments/webhooks.py file around lines 192-194
wc -l payments/webhooks.pyRepository: GooeyAI/gooey-server
Length of output: 88
🏁 Script executed:
#!/bin/bash
# Read the specific section of payments/webhooks.py around lines 192-194
sed -n '185,205p' payments/webhooks.pyRepository: GooeyAI/gooey-server
Length of output: 713
🏁 Script executed:
#!/bin/bash
# Check the type annotation for charged_amount in the set_workspace_subscription function
sed -n '265,290p' payments/webhooks.pyRepository: GooeyAI/gooey-server
Length of output: 771
Normalize charged_amount to an int in smallest currency units.
round(Decimal(stripe_sub.plan.amount_decimal) * amount) returns a Decimal, but the set_workspace_subscription function expects charged_amount: int (line 271), and the Subscription model defines it as a PositiveIntegerField. Per Stripe's API, plan.amount_decimal is a decimal string representing the smallest currency unit (cents for USD). To keep types and units consistent with other payment paths like handle_invoice_paid (which passes Stripe's integer amounts directly), cast explicitly:
- amount = int(stripe_sub.quantity)
- charged_amount = round(Decimal(stripe_sub.plan.amount_decimal) * amount)
+ amount = int(stripe_sub.quantity)
+ # amount_decimal is in the smallest currency unit; keep charged_amount in cents.
+ unit_amount = Decimal(stripe_sub.plan.amount_decimal)
+ charged_amount = int(unit_amount * amount)🤖 Prompt for AI Agents
In payments/webhooks.py around lines 192-194, charged_amount is currently a
Decimal produced by round(Decimal(stripe_sub.plan.amount_decimal) * amount) but
downstream expects an int in the smallest currency unit; replace the Decimal
result with an explicit integer (e.g. apply proper rounding then cast to int) so
charged_amount is an int representing smallest currency units before passing to
set_workspace_subscription/Subscription.
1906f32 to
6ab6504
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
recipes/VideoBots.py (1)
22-63: Remove duplicateUserErrorimport
UserErroris already imported at line 22; the second import at line 63 is redundant and flagged by Ruff F811. Please drop the line-63 import and keep just one.-from daras_ai_v2.exceptions import UserErrorpayments/models.py (1)
40-80: AlignSubscriptiondefaults with migration and fix apostrophe in help_textTwo issues here:
auto_recharge_enableddefault mismatch (model vs migration)
Subscription.auto_recharge_enabledis defined withdefault=Truein this model, while migration0007_subscription_amount_subscription_charged_amount_and_moresets the DB field default toFalse.- This divergence means new
Subscription()instances created in Python will default toTrueeven though the DB default isFalse. That undermines the “auto‑recharge off by default” intent for new non‑enterprise users.- Recommend updating the model field to
default=Falseto match the migration and intended behavior.Unicode apostrophe in
charged_amounthelp_text
- The help_text uses
currency’swith a RIGHT SINGLE QUOTATION MARK, which Ruff flags (RUF001) and can be a nuisance in diffs.- Replace it with a standard ASCII apostrophe.
- auto_recharge_enabled = models.BooleanField(default=True) + auto_recharge_enabled = models.BooleanField(default=False)- charged_amount = models.PositiveIntegerField( - help_text="The charged dollar amount in the currency’s smallest unit.<br>" + charged_amount = models.PositiveIntegerField( + help_text="The charged dollar amount in the currency's smallest unit.<br>" "E.g. for 10 USD, this would be of 1000 (that is, 1000 cents).<br>" "<a href='https://stripe.com/docs/currencies'>Learn More</a>", default=0, )daras_ai_v2/billing.py (1)
663-680: Addon section is not gated for non-enterprise users.The
render_addon_sectioncall at line 57 lacks protection for non-enterprise workspaces. According to PR objectives, addon sections should only be shown whenworkspace.allow_credit_topups()returns true. However, this method does not exist in the codebase, and no guard is currently in place. Either implement theallow_credit_topups()method on the Workspace model and gate this call, or verify if a different gating mechanism was intended.
♻️ Duplicate comments (7)
recipes/VideoBots.py (1)
1948-1955: Fix “setup” → “set up” in WhatsApp enterprise copyThe message still uses the verb as “setup”; it should be “set up”, and this was already flagged earlier.
- gui.write( - "As a premium customer, please contact us to setup a managed number" - ) + gui.write( + "As a premium customer, please contact us to set up a managed number" + )usage_costs/migrations/0038_alter_modelpricing_model_name.py (1)
245-287: Deduplicate repeatedmodel_namechoices and normalize labelsThis choices list still has duplicate keys with conflicting labels (e.g.,
dream_shaper,sd_2,sd_1_5,dall_e,gpt_image_1,nano_banana,jack_qiao,rodent_diffusion_1_5appear multiple times, withdall_eusing both “DALL·E 2 (OpenAI)” and “Dall-E (OpenAI)”). Django will only honor the first entry per key, so the later ones are effectively dead and add confusion.Consider pruning duplicates and keeping a single canonical label per key (matching whatever you chose in 0037), then regenerating this migration if needed.
payments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.py (1)
21-27: Normalize apostrophe incharged_amounthelp_text (match model)The help_text here still uses a Unicode right single quote in
currency’s, which Ruff flags and which can cause minor encoding headaches. It also diverges from the ASCII apostrophe we’d like inpayments/models.py.- help_text="The charged dollar amount in the currency’s smallest unit.<br>E.g. for 10 USD, this would be of 1000 (that is, 1000 cents).<br><a href='https://stripe.com/docs/currencies'>Learn More</a>", + help_text="The charged dollar amount in the currency's smallest unit.<br>E.g. for 10 USD, this would be of 1000 (that is, 1000 cents).<br><a href='https://stripe.com/docs/currencies'>Learn More</a>",payments/webhooks.py (2)
175-185: Avoidassertfor missing Stripe plan; log and return insteadIn the fallback path,
PricingPlan.get_by_stripe_product(product)can still returnNonefor legacy or unknown Stripe products;assert plan is not Nonewill then raise, crash the webhook, and cause Stripe to keep retrying. This was already noted earlier.Replace the assertion with a warning + early return so the webhook still 2xx’s:
- except KeyError: - product = stripe.Product.retrieve( - stripe_sub.plan.product, expand=["default_price"] - ) - plan = PricingPlan.get_by_stripe_product(product) - assert plan is not None, f"Plan for product {product.id} not found" + except KeyError: + product = stripe.Product.retrieve( + stripe_sub.plan.product, expand=["default_price"] + ) + plan = PricingPlan.get_by_stripe_product(product) + if plan is None: + logger.warning( + f"Stripe subscription {stripe_sub.id} has unmapped product {product.id}; ignoring update" + ) + return
192-202: Normalizecharged_amountto anintin smallest currency units
round(Decimal(stripe_sub.plan.amount_decimal) * amount)yields aDecimal, butcharged_amountis treated as aninteverywhere else (PositiveIntegerField onSubscription,add_balance_for_paymentsignature, and budget calculations). To keep types and units consistent, cast explicitly after rounding.- amount = int(stripe_sub.quantity) - charged_amount = round(Decimal(stripe_sub.plan.amount_decimal) * amount) + amount = int(stripe_sub.quantity) + unit_amount = Decimal(stripe_sub.plan.amount_decimal) # smallest currency unit + charged_amount = int(unit_amount * amount)In Stripe's Python API, is `plan.amount_decimal` documented as a decimal string in the currency's smallest unit, and is it safe to treat `unit_amount_decimal * quantity` as an integer number of cents when casting to `int` after appropriate rounding?daras_ai_v2/billing.py (2)
281-287: Fix misleading docstring.The docstring claims the function returns a selected tier key, but the function doesn't return anything. Tier selection was moved to
_render_plan_pricing.Apply this diff to correct the docstring:
def _render_plan_details(plan: PricingPlan): - """Render plan details and return selected tier key if plan has tiers""" + """Render plan description and footer"""
440-445: Missing None check forselected_tier.Line 444 accesses
selected_tier.monthly_chargewithout verifyingselected_tieris not None. While the condition checksplan.tiersandcurrent_tier, it doesn't checkselected_tier. If the payment provider is not Stripe or tier selection fails,selected_tierwill be None and cause an AttributeError.Apply this diff to add the None check:
if plan > current_plan or ( plan == current_plan and plan.tiers and current_tier + and selected_tier and (selected_tier.monthly_charge > current_tier.monthly_charge) ):
🧹 Nitpick comments (1)
payments/plans.py (1)
394-398: Consider defining a custom exception or moving message to exception class.The static analysis tool flags TRY003: using long messages directly in raise statements is discouraged. While not critical, defining a custom exception class or using a shorter message would align with best practices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app_users/migrations/0029_alter_appusertransaction_plan.py(1 hunks)app_users/tasks.py(0 hunks)daras_ai_v2/billing.py(17 hunks)payments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.py(1 hunks)payments/models.py(4 hunks)payments/plans.py(10 hunks)payments/utils.py(1 hunks)payments/webhooks.py(5 hunks)recipes/VideoBots.py(2 hunks)routers/paypal.py(1 hunks)usage_costs/migrations/0038_alter_modelpricing_model_name.py(1 hunks)
💤 Files with no reviewable changes (1)
- app_users/tasks.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,js,ts,tsx,java,cs,cpp,c,go,rb,php}
📄 CodeRabbit inference engine (.cursor/rules/devs-rules.mdc)
Format code in reverse topological order: place the main() function at the top and dependencies below it
Files:
payments/models.pypayments/webhooks.pypayments/plans.pyapp_users/migrations/0029_alter_appusertransaction_plan.pyusage_costs/migrations/0038_alter_modelpricing_model_name.pypayments/utils.pyrouters/paypal.pydaras_ai_v2/billing.pypayments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.pyrecipes/VideoBots.py
🧬 Code graph analysis (7)
payments/models.py (1)
payments/plans.py (3)
PricingPlan(81-506)PricingTier(23-29)from_sub(338-341)
payments/plans.py (2)
payments/utils.py (1)
make_stripe_recurring_plan(4-31)daras_ai_v2/paypal.py (2)
Product(237-242)create(82-92)
app_users/migrations/0029_alter_appusertransaction_plan.py (1)
app_users/migrations/0026_alter_appusertransaction_plan.py (1)
Migration(6-18)
usage_costs/migrations/0038_alter_modelpricing_model_name.py (2)
usage_costs/migrations/0037_alter_modelpricing_model_name.py (1)
Migration(6-298)usage_costs/migrations/0033_alter_modelpricing_model_name_and_more.py (1)
Migration(6-297)
routers/paypal.py (1)
payments/plans.py (3)
PricingPlan(81-506)get_by_key(394-398)supports_paypal(403-404)
daras_ai_v2/billing.py (2)
payments/plans.py (10)
PricingPlan(81-506)PricingTier(23-29)get_active_monthly_charge(69-73)get_active_credits(63-67)get_pricing_title(49-54)label(28-29)from_sub(338-341)get_pricing_caption(56-61)get_stripe_line_item(406-427)supports_stripe(400-401)payments/models.py (2)
get_tier(133-138)Subscription(37-403)
recipes/VideoBots.py (1)
daras_ai_v2/exceptions.py (1)
UserError(58-65)
🪛 Ruff (0.14.7)
payments/models.py
46-46: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
payments/plans.py
78-78: Avoid specifying long messages outside the exception class
(TRY003)
398-398: Avoid specifying long messages outside the exception class
(TRY003)
408-408: Avoid specifying long messages outside the exception class
(TRY003)
app_users/migrations/0029_alter_appusertransaction_plan.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-33: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
usage_costs/migrations/0038_alter_modelpricing_model_name.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-298: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
payments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-50: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
26-26: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
recipes/VideoBots.py
63-63: Redefinition of unused UserError from line 22
Remove definition: UserError
(F811)
⏰ 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). (2)
- GitHub Check: test (3.10.12, 1.8.3)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (27)
payments/models.py (1)
133-138:get_tiermapping from cents to dollars is consistent withPricingTierUsing
round(self.charged_amount / 100)to map cents → integer dollars formonthly_chargeis consistent with how budgets are computed elsewhere (charged_amount in cents, budgets in dollars). Givencharged_amountshould be a multiple of 100 for subscriptions, this is safe.app_users/migrations/0029_alter_appusertransaction_plan.py (1)
12-31: Plan choices update forAppUserTransactionlooks consistentThe updated
planchoices (includingStandardas 8 andFreefor 3) align with the subscription plan set used elsewhere; migration wiring looks correct.routers/paypal.py (1)
122-130: Improved PayPal subscription validation looks goodSeparating invalid plan keys (KeyError) from “plan does not support PayPal” is clearer for clients and keeps the control flow simple.
payments/webhooks.py (1)
264-288: Newamount/charged_amountplumbed into subscriptions correctlyPassing
amountandcharged_amountintoset_workspace_subscriptionand persisting them on theSubscriptionbeforefull_clean()is consistent with the new fields and withSubscription.ensure_default_auto_recharge_paramsexpecting credits and cents. The cloning pattern aroundold_subremains unchanged. Call sites pass these parameters where relevant—only the Stripe subscription creation handler passes calculated values, while cancellations and downgrades use the default zeros.payments/plans.py (11)
23-30: LGTM!The
PricingTierNamedTuple is well-designed with clear fields and a helpful label property for display.
46-79: LGTM!The tier-aware methods correctly handle both tiered and non-tiered plans with proper fallback logic. The
get_default_tier()appropriately raises an error for plans without tiers.
165-199: LGTM!The STARTER plan updates correctly position it as a free tier with clear feature descriptions and appropriate pricing display.
201-236: LGTM!The STANDARD plan is well-structured with clear tier definitions and appropriate feature descriptions.
238-274: LGTM!The BUSINESS plan updates correctly reference the new plan hierarchy and feature set.
276-316: LGTM!The ENTERPRISE plan updates add full-width display support with a well-structured two-column layout.
318-325: LGTM!The plan comparison logic correctly uses default pricing (no tier specified) for comparing plans at their base level.
338-341: LGTM!The updated
from_submethod correctly handles None subscriptions by returning STARTER as the default plan.
351-385: LGTM!The tier-aware Stripe product matching logic correctly handles both tiered and non-tiered plans with appropriate name and price validation.
406-427: LGTM!The tier-aware line item generation correctly derives pricing and product IDs based on the specified tier.
439-466: LGTM!The tier-aware Stripe product ID generation correctly builds product names and pricing for both tiered and non-tiered plans.
daras_ai_v2/billing.py (12)
82-142: LGTM!The current plan rendering correctly retrieves and displays tier-aware pricing information from the subscription.
151-192: LGTM!The plan rendering logic correctly filters and organizes plans based on workspace type and plan attributes.
194-239: LGTM!The full-width plan rendering correctly captures tier selection and propagates it through the action button rendering for both desktop and mobile layouts.
242-278: LGTM!The compact plan rendering correctly captures tier selection and propagates it through the action button rendering.
302-330: LGTM!The tier selection and pricing display logic correctly handles both tiered and non-tiered plans with appropriate defaults from the current subscription.
333-394: LGTM!The action button logic correctly handles tier context for all subscription states (current plan tier changes, upgrades, downgrades, and new subscriptions).
474-539: LGTM!The upgrade subscription logic correctly handles tier-aware pricing and propagates tier context through the upgrade flow. The code assumes
workspace.subscriptionexists (validated by callers) and safely handles None tiers via the tier-aware pricing methods.
541-571: LGTM!The subscription creation logic correctly propagates tier context to Stripe flows and appropriately documents that PayPal doesn't support tiers.
574-578: LGTM!The refactored
fmt_pricefunction is more flexible and reusable by accepting a numeric price directly.
581-649: LGTM!The subscription change logic correctly handles tier-aware updates, including no-op detection and proper propagation of tier context to Stripe line items.
801-843: LGTM!The Stripe subscription button correctly displays tier-aware pricing and propagates tier context to the subscription creation flow.
846-901: LGTM!The Stripe subscription creation correctly propagates tier context through both direct subscription and checkout flows, with appropriate metadata for webhook handling.
| cents_per_month = amount * 100 | ||
| price_data = { | ||
| "currency": "usd", | ||
| "unit_amount_decimal": round(cents_per_month / credits, 4), | ||
| "unit_amount_decimal": round(cents_per_month / credits, 12), | ||
| "recurring": {"interval": "month"}, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Stripe unit_amount_decimal field decimal places maximum specification
💡 Result:
Stripe's unit_amount_decimal accepts a decimal string in cents (or local equivalent) with a maximum of 12 decimal places. [1][2]
Sources:
[1] stripe.dev — PriceCreateParams / unit_amount_decimal.
[2] async-stripe (docs.rs) — unit_amount_decimal description.
🏁 Script executed:
# Check the structure of payments/utils.py to verify reverse topological order guideline
head -50 payments/utils.pyRepository: GooeyAI/gooey-server
Length of output: 1382
🏁 Script executed:
# Get full file to understand the structure and identify main functions
wc -l payments/utils.pyRepository: GooeyAI/gooey-server
Length of output: 84
🌐 Web query:
Stripe unit_amount_decimal JSON float vs decimal string format requirement
💡 Result:
Short answer: Stripe expects unit_amount_decimal as a decimal string in the JSON (not a JSON float). Send it as a string with at most 12 decimal places (and only one of unit_amount or unit_amount_decimal may be set). Cite: Stripe API docs. [1][2]
Sources
- Prices / Price object (unit_amount_decimal = "decimal string"). [1]
- Payment Link / Price fields (unit_amount_decimal = nullable decimal string). [2]
🏁 Script executed:
# Check if there's any JSON serialization or Stripe API calls in the codebase
rg -A 5 "unit_amount_decimal" --type pyRepository: GooeyAI/gooey-server
Length of output: 2211
🏁 Script executed:
# Check how the price_data dict is being serialized/sent to Stripe
rg -B 5 -A 5 "make_stripe_recurring_plan" --type pyRepository: GooeyAI/gooey-server
Length of output: 1800
🏁 Script executed:
# Look for JSON encoding or Stripe API calls that might serialize this
rg "json\.dumps|stripe\." --type py -A 3 | head -50Repository: GooeyAI/gooey-server
Length of output: 2572
unit_amount_decimal must be sent as a decimal string, not a float
Stripe's API requires unit_amount_decimal as a string with up to 12 decimal places. The current code sends a float from round(cents_per_month / credits, 12), which doesn't match the specification. Convert to string explicitly:
"unit_amount_decimal": str(round(cents_per_month / credits, 12)),Alternatively, use Decimal for cleaner string representation and better precision handling:
from decimal import Decimal
"unit_amount_decimal": str(Decimal(cents_per_month) / Decimal(credits)),🤖 Prompt for AI Agents
In payments/utils.py around lines 17 to 21, the code currently sets
unit_amount_decimal using a float from round(cents_per_month / credits, 12);
change this to send a string as required by Stripe — either convert the rounded
value to a string (e.g., str(round(..., 12))) or, preferably, import Decimal
from decimal and compute the division with Decimals then convert to string
(e.g., str(Decimal(cents_per_month) / Decimal(credits))) so unit_amount_decimal
is a decimal string with correct precision.
don't crash in webhook handling code
…ck on plan.credits / plan.monthly_charge) so we don't accidentally use wrong values for a tiered plan
nikochiko
force-pushed
the
subscription-plan-standard
branch
from
027beaf to
c273a77
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
recipes/VideoBots.py (1)
1967-1967: Fix grammatical error: "setup" → "set up".When used as a verb phrase, "set up" should be two words. Line 2008 correctly uses "set up" in a similar context.
🔎 Proposed fix
- "As a premium customer, please contact us to setup a managed number" + "As a premium customer, please contact us to set up a managed number"payments/utils.py (1)
17-21: ** Convertunit_amount_decimalto string as required by Stripe API.**Stripe's
unit_amount_decimalfield must be a decimal string, not a float. The current code passes the result ofround(), which is a float. This issue was previously flagged and remains unresolved.Apply the fix from the previous review:
"unit_amount_decimal": str(round(cents_per_month / credits, 12)),Or use
Decimalfor better precision handling:from decimal import Decimal "unit_amount_decimal": str(Decimal(cents_per_month) / Decimal(credits)),Based on the previous review discussion and Stripe API documentation.
daras_ai_v2/billing.py (1)
281-286: ** Update misleading docstring.**The docstring states the function returns a "selected tier key if plan has tiers," but
_render_plan_detailsdoesn't return anything. Tier selection was moved to_render_plan_pricing.Update to accurately reflect the function's behavior:
-def _render_plan_details(plan: PricingPlan): - """Render plan details and return selected tier key if plan has tiers""" +def _render_plan_details(plan: PricingPlan): + """Render plan description and footer"""payments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.py (1)
24-27: ** Replace Unicode apostrophe in help_text with ASCII quote.**Line 26 contains a Unicode right single quotation mark (') in the
hrefattribute instead of a standard ASCII apostrophe ('). This can cause encoding issues.- help_text="The charged dollar amount in the currency's smallest unit.<br>E.g. for 10 USD, this would be of 1000 (that is, 1000 cents).<br><a href='https://stripe.com/docs/currencies'>Learn More</a>", + help_text="The charged dollar amount in the currency's smallest unit.<br>E.g. for 10 USD, this would be of 1000 (that is, 1000 cents).<br><a href='https://stripe.com/docs/currencies'>Learn More</a>",As flagged by static analysis (Ruff RUF001).
payments/models.py (1)
45-50: ** Replace Unicode apostrophe in help_text with ASCII quote.**Line 48 contains a Unicode right single quotation mark (') in the
hrefattribute. Replace with standard ASCII apostrophe:- "<a href='https://stripe.com/docs/currencies'>Learn More</a>", + "<a href='https://stripe.com/docs/currencies'>Learn More</a>",As flagged by static analysis (Ruff RUF001).
payments/webhooks.py (1)
192-194: ** Castcharged_amounttointfor type consistency.**
round(Decimal(...) * amount)returns aDecimal, butset_workspace_subscriptionexpectscharged_amount: int(line 271), and theSubscription.charged_amountfield is aPositiveIntegerField. Cast explicitly toint:amount = int(stripe_sub.quantity) -charged_amount = round(Decimal(stripe_sub.plan.amount_decimal) * amount) +charged_amount = int(round(Decimal(stripe_sub.plan.amount_decimal) * amount))Or more directly:
-charged_amount = round(Decimal(stripe_sub.plan.amount_decimal) * amount) +charged_amount = int(Decimal(stripe_sub.plan.amount_decimal) * amount)Per the previous review and type signatures.
🧹 Nitpick comments (1)
payments/webhooks.py (1)
175-184: Consider logging when falling back to product-based plan lookup.The fallback from metadata-based to product-based plan resolution works correctly, but it would be helpful to log when this fallback occurs (before line 180) to track how often legacy subscriptions are encountered.
except KeyError: + logger.info(f"Plan key not found in metadata, falling back to product lookup for subscription {stripe_sub.id}") product = stripe.Product.retrieve(
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app_users/migrations/0029_alter_appusertransaction_plan.pyapp_users/tasks.pydaras_ai_v2/billing.pypayments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.pypayments/models.pypayments/plans.pypayments/utils.pypayments/webhooks.pyrecipes/VideoBots.pyrouters/paypal.py
💤 Files with no reviewable changes (1)
- app_users/tasks.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,js,ts,tsx,java,cs,cpp,c,go,rb,php}
📄 CodeRabbit inference engine (.cursor/rules/devs-rules.mdc)
Format code in reverse topological order: place the main() function at the top and dependencies below it
Files:
routers/paypal.pypayments/webhooks.pypayments/plans.pypayments/models.pydaras_ai_v2/billing.pyrecipes/VideoBots.pypayments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.pypayments/utils.pyapp_users/migrations/0029_alter_appusertransaction_plan.py
🧬 Code graph analysis (6)
routers/paypal.py (1)
payments/plans.py (3)
PricingPlan(81-506)get_by_key(394-398)supports_paypal(403-404)
payments/webhooks.py (1)
payments/plans.py (3)
PricingPlan(81-506)get_by_key(394-398)get_by_stripe_product(351-385)
payments/plans.py (2)
payments/utils.py (1)
make_stripe_recurring_plan(4-31)daras_ai_v2/paypal.py (2)
Product(237-242)create(82-92)
payments/models.py (1)
payments/plans.py (3)
PricingPlan(81-506)PricingTier(23-29)from_sub(338-341)
daras_ai_v2/billing.py (3)
payments/plans.py (9)
PricingPlan(81-506)PricingTier(23-29)get_active_monthly_charge(69-73)get_active_credits(63-67)get_pricing_title(49-54)label(28-29)get_pricing_caption(56-61)get_stripe_line_item(406-427)supports_stripe(400-401)payments/models.py (2)
get_tier(133-138)Subscription(37-403)app_users/models.py (1)
PaymentProvider(85-87)
recipes/VideoBots.py (1)
daras_ai_v2/exceptions.py (1)
UserError(58-65)
🪛 Ruff (0.14.10)
payments/plans.py
78-78: Avoid specifying long messages outside the exception class
(TRY003)
398-398: Avoid specifying long messages outside the exception class
(TRY003)
408-408: Avoid specifying long messages outside the exception class
(TRY003)
payments/models.py
46-46: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
recipes/VideoBots.py
63-63: Redefinition of unused UserError from line 22
Remove definition: UserError
(F811)
payments/migrations/0007_subscription_amount_subscription_charged_amount_and_more.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-50: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
26-26: String contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF001)
app_users/migrations/0029_alter_appusertransaction_plan.py
8-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
12-33: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ 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). (1)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (11)
routers/paypal.py (1)
123-129: LGTM! Improved error handling for plan validation.The separation of key validation and PayPal support checks provides clearer, more specific error messages to users. The try/except pattern properly handles the
KeyErrorfromget_by_key.payments/models.py (2)
103-111: LGTM! Tier-aware default computation infull_clean.The logic correctly computes
amountandcharged_amountfrom the plan's tier-aware accessors when auto-recharge is enabled, providing sensible defaults when these fields are not yet populated.
133-138: LGTM! Clean tier reconstruction from stored fields.The
get_tier()method properly reconstructs aPricingTierfrom the storedamount(credits) andcharged_amount(converting cents to dollars), returningNonewhen either field is missing.app_users/migrations/0029_alter_appusertransaction_plan.py (1)
1-33: LGTM! Migration correctly updates plan choices.The migration properly updates the
AppUserTransaction.planfield choices to align with the new tiered pricing system, including the new Standard plan (value 8).Note: The Ruff ClassVar warnings are false positives for Django migration files and can be safely ignored.
payments/webhooks.py (1)
100-107: LGTM! Proper error handling for plan key lookup.The try/except block correctly handles
KeyErrorfromget_by_keywhen metadata is missing or invalid, capturing the exception with Sentry for monitoring while allowing the webhook to continue processing.payments/plans.py (6)
23-29: LGTM! CleanPricingTierdefinition.The
PricingTierNamedTuple provides a simple, immutable structure for tier data with a usefullabelproperty for display.
49-78: LGTM! Well-designed tier-aware accessor methods.The tier-aware methods provide a clean API for accessing pricing data, falling back to default values when no tier is specified. The
get_default_tier()method appropriately raises an error for non-tiered plans.
201-236: LGTM! Comprehensive STANDARD plan definition with tiered pricing.The STANDARD plan is well-structured with 6 pricing tiers ranging from $25 to $400/month, clear feature descriptions, and appropriate positioning between Free and Business plans.
406-427: LGTM! Tier-aware line item creation.The method correctly uses tier-specific or default credits and monthly_charge values, maintaining backward compatibility for non-tiered plans.
439-466: LGTM! Tier-aware Stripe product ID resolution.The method properly constructs tier-specific product names (e.g., "Standard - 2,000 Credits for $25/month") and creates new products when needed, ensuring each tier has its own Stripe product.
351-385: Edge case in tier matching whenproduct.default_priceis missing deserves clarification.The tier matching logic uses both product name and price for matching, which is comprehensive. However, when
product.default_priceis None, the conditionproduct_price is None or product_price == expected_priceallows a name-only match.While products created by
get_stripe_product_id()always includedefault_price_data, and callers likepayments/webhooks.pyexplicitly expand it during retrieval, this path could be triggered if:
- External Stripe products are added without a default price
- The API expansion fails silently
If name-only matching is intentional as a fallback, add a comment clarifying this. Otherwise, consider asserting that
product.default_priceexists, or return None if pricing cannot be verified.The early return on first tier match is correct—each tier has a unique label (based on distinct credit amounts), making the product name unique per tier.
| if plan > current_plan or ( | ||
| plan == current_plan | ||
| and plan.tiers | ||
| and current_tier | ||
| and selected_tier.monthly_charge > current_tier.monthly_charge | ||
| ): |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read the specific lines from billing.py to verify the review comment
if [ -f "daras_ai_v2/billing.py" ]; then
echo "=== Lines 435-450 to see function signature and context ==="
sed -n '435,450p' daras_ai_v2/billing.py | cat -n
else
echo "File not found"
fiRepository: GooeyAI/gooey-server
Length of output: 819
Add None check for selected_tier before accessing its attribute at line 444.
The condition accesses selected_tier.monthly_charge without verifying selected_tier is not None, despite the function signature (line 435) allowing it to be None. When selected_tier is None and the other conditions evaluate to true, this raises AttributeError.
Add the check:
and selected_tier
and selected_tier.monthly_charge > current_tier.monthly_charge🤖 Prompt for AI Agents
In daras_ai_v2/billing.py around lines 440 to 445, the conditional compares
selected_tier.monthly_charge but the function signature allows selected_tier to
be None; add an explicit None check for selected_tier before accessing its
monthly_charge. Modify the condition to include "and selected_tier" (or
equivalent truthy check) immediately before comparing monthly_charge so
selected_tier is guaranteed non-None when selected_tier.monthly_charge is read.
| ) | ||
| from daras_ai_v2.embedding_model import EmbeddingModels | ||
| from daras_ai_v2.enum_selector_widget import enum_selector | ||
| from daras_ai_v2.exceptions import UserError |
There was a problem hiding this comment.
Remove duplicate import.
UserError is already imported at line 22. This duplicate import is unnecessary and flagged by static analysis.
🔎 Proposed fix
-from daras_ai_v2.exceptions import UserError📝 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.
| from daras_ai_v2.exceptions import UserError |
🧰 Tools
🪛 Ruff (0.14.10)
63-63: Redefinition of unused UserError from line 22
Remove definition: UserError
(F811)
🤖 Prompt for AI Agents
In recipes/VideoBots.py around line 63, there's a duplicate import of UserError
(already imported at line 22); remove the redundant "from daras_ai_v2.exceptions
import UserError" at line 63 so the file has only the single import at line 22
and run linters to confirm the duplicate-import warning is resolved.
Q/A checklist
How to check import time?
You can visualize this using tuna:
To measure import time for a specific library:
To reduce import times, import libraries that take a long time inside the functions that use them instead of at the top of the file:
Legal Boilerplate
Look, I get it. The entity doing business as “Gooey.AI” and/or “Dara.network” was incorporated in the State of Delaware in 2020 as Dara Network Inc. and is gonna need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Dara Network Inc can use, modify, copy, and redistribute my contributions, under its choice of terms.
Note
Adds a new tiered Standard plan with Stripe integration, overhauls billing UI/flows, records subscription amounts, and restricts credit topups/auto‑recharge to eligible workspaces.
PricingPlan.STANDARDwith multiplePricingTiers and tier-aware helpers (get_active_*,get_default_tier).PricingPlan.from_subacceptsNone.daras_ai_v2/billing.py)STANDARDon team workspaces.allow_credit_topups().Subscription: addamount,charged_amount; defaultauto_recharge_enabled=False; addget_tier().Standard) and new fields; largeusage_costsmodel name choices refresh.amountandcharged_amount.workspace.allow_credit_topups(); compute budgets from plan if needed; precision tweak for Stripe per-credit pricing.workspaces/models.py)allow_credit_topups()to permit topups only for Enterprise or workspaces with prior topups.save_stripe_default_payment_methodto newset_workspace_subscriptionsignature.disable_auto_recharge_for_restricted_workspaces.pyto turn off auto‑recharge where not allowed.Written by Cursor Bugbot for commit 1906f32. This will update automatically on new commits. Configure here.