Allow credit usage without active plan#1575
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughReplaces throw-on-missing Stripe credit configuration with a data-driven fallback (Solo plan lookup); removes Stripe audit logging and plan-cache purge side effects; centralizes Stripe/Bento configuration checks; surfaces Changes
Sequence Diagram(s)sequenceDiagram
participant Stripe as Stripe Webhook
participant Handler as stripe_event Handler
participant Credits as Credits Util
participant DB as Database (plans)
Stripe->>Handler: deliver event (customerId, product_id)
Handler->>Handler: extract identifiers, decide flow
Handler->>Credits: getCreditTopUpProductId(customerId, product_id)
Credits->>DB: look up product -> find credit_id
alt product_id missing or credit_id not found
Credits->>Credits: call getFallbackCreditProductId(customerId, fetchPlan)
Credits->>DB: query `plans` for "Solo" plan
DB-->>Credits: return Solo plan.credit_id
Credits-->>Handler: return fallback credit_id
else product_id maps to credit
DB-->>Credits: return mapped credit_id
Credits-->>Handler: return mapped credit_id
end
Handler->>Handler: continue top-up processing using returned credit_id
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27587ce270
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const hasActivePlan = org?.stripe_info?.status === 'succeeded' | ||
| const totalStats = await getTotalStats(c, orgId) | ||
| const currentPlanName = await getCurrentPlanNameOrg(c, orgId) | ||
| if (!totalStats) { | ||
| return false | ||
| } | ||
|
|
||
| const { data: currentPlan, error: currentPlanError } = await supabaseAdmin(c) | ||
| .from('plans') | ||
| .select('*') | ||
| .eq('name', currentPlanName) | ||
| .single() | ||
| if (currentPlanError) { | ||
| cloudlogErr({ requestId: c.get('requestId'), message: 'currentPlanError', error: currentPlanError }) | ||
| let currentPlanName: string | null = null | ||
| let currentPlan: Database['public']['Tables']['plans']['Row'] | null = null | ||
| if (hasActivePlan) { | ||
| currentPlanName = await getCurrentPlanNameOrg(c, orgId) | ||
| const { data, error: currentPlanError } = await supabaseAdmin(c) | ||
| .from('plans') | ||
| .select('*') | ||
| .eq('name', currentPlanName) | ||
| .single() | ||
| if (currentPlanError) { | ||
| cloudlogErr({ requestId: c.get('requestId'), message: 'currentPlanError', error: currentPlanError }) | ||
| } | ||
| currentPlan = data ?? null | ||
| } |
There was a problem hiding this comment.
The new hasActivePlan gate means getCurrentPlanNameOrg() is only called when stripe_info.status === 'succeeded'. For orgs on the free/default plan (no Stripe subscription or status), currentPlan stays null and later limits are coerced to 0, so any usage becomes overage and triggers exceeded flags/upgrade notifications. This changes behavior for free-plan orgs that should still get the Solo limits. Consider always fetching the plan name (it already falls back to Solo) and only skip paid-plan checks/notifications based on status, rather than skipping the plan lookup entirely.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This pull request enables organizations to use credits for service usage even when they don't have an active Stripe subscription plan. The changes implement a "credits-only" usage model with a Solo plan fallback for credit product configuration.
Changes:
- Modified database functions to allow service usage when credits are available (can_use_more returns true if credits exist)
- Added Solo plan fallback logic in backend code when retrieving credit product IDs for organizations without configured Stripe plans
- Updated plan validation to treat available credits as sufficient authorization for actions
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| supabase/migrations/20260127121000_allow_credits_without_plan.sql | Updates get_orgs_v7, is_paying_and_good_plan_org, and is_paying_and_good_plan_org_action functions to check for available credits as an alternative to active plans |
| supabase/functions/_backend/utils/plans.ts | Modifies applyCreditsForMetric to log when plan context is missing but billing cycle exists; updates userAbovePlan to handle credit-only organizations and adds early return logic for organizations without active plans |
| supabase/functions/_backend/utils/pg.ts | Enhances buildPlanValidationExpression to check for available credits before requiring active subscription status |
| supabase/functions/_backend/triggers/stripe_event.ts | Adds Solo plan fallback when getCreditTopUpProductIdFromCustomer cannot find organization's credit product configuration |
| supabase/functions/_backend/private/credits.ts | Implements Solo plan fallback in getCreditTopUpProductId when credit product lookup fails for organizations |
| let fallbackPlan: { credit_id: string | null } | undefined | ||
| try { | ||
| ;[fallbackPlan] = await drizzleClient | ||
| .select({ credit_id: schema.plans.credit_id }) | ||
| .from(schema.plans) | ||
| .where(eq(schema.plans.name, 'Solo')) | ||
| .limit(1) | ||
| } | ||
| catch (error) { | ||
| cloudlog({ | ||
| requestId: c.get('requestId'), | ||
| message: 'credit_fallback_plan_missing', | ||
| customerId, | ||
| error, | ||
| }) | ||
| } | ||
| if (!fallbackPlan?.credit_id) | ||
| throw simpleError('credit_product_not_configured', 'Credit product is not configured') | ||
| return fallbackPlan.credit_id |
There was a problem hiding this comment.
This fallback logic for fetching the Solo plan's credit_id is duplicated in four places across two files (twice in this file, twice in credits.ts). Consider extracting this into a shared helper function like getFallbackCreditProductId(c: Context, customerId: string) in a common utility module to improve maintainability and reduce the risk of inconsistencies.
| cloudlogErr({ | ||
| requestId: c.get('requestId'), | ||
| message: 'credit_fallback_plan_missing', | ||
| customerId, | ||
| error: fallbackError, | ||
| }) |
There was a problem hiding this comment.
The logging approach differs between this file and stripe_event.ts. Here, cloudlogErr is used for logging the fallback plan query failure, while stripe_event.ts uses cloudlog (non-error). For consistency, consider using the same logging level across both files since this represents the same fallback scenario.
| @@ -84,6 +84,15 @@ async function applyCreditsForMetric( | |||
| credit_step_id: null, | |||
| } | |||
| } | |||
There was a problem hiding this comment.
When there's no billing cycle (billingCycle is null or missing subscription_anchor_start/end), credits cannot be applied and all overage is returned as unpaid. However, the PR's goal is to allow credit usage without an active plan. Without an active plan, organizations might not have a billing cycle configured (see get_cycle_info_org which depends on stripe_info.subscription_anchor_start). This creates a situation where organizations with only credits cannot actually use them for overage, defeating the purpose of this PR. Consider either creating a default billing cycle for credit-only organizations or modifying the credit application logic to work without billing cycle information.
riderx
force-pushed
the
riderx/allow-credit-no-plan
branch
from
27587ce to
843ad87
Compare
![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
🤖 Fix all issues with AI agents
In `@supabase/functions/_backend/utils/plans.ts`:
- Around line 110-111: The TypeScript error comes from passing
resolvedBillingCycle.subscription_anchor_start and subscription_anchor_end
(typed as string | null) into RPC params that require string; update the
assignment of p_billing_cycle_start and p_billing_cycle_end to assert non-null
(e.g., use the non-null assertion on
resolvedBillingCycle.subscription_anchor_start! and
resolvedBillingCycle.subscription_anchor_end!) or alternatively change
getDefaultBillingCycleRange to return a type with non-nullable fields and use
that type for resolvedBillingCycle so the properties are typed as string,
ensuring the RPC parameters p_billing_cycle_start and p_billing_cycle_end
receive non-nullable strings.
🧹 Nitpick comments (1)
supabase/functions/_backend/utils/plans.ts (1)
43-50: Consider a non-nullable return type forgetDefaultBillingCycleRange.The helper always returns non-null ISO strings, but returns
BillingCycleInfowhich has nullable fields. A dedicated non-nullable interface would provide type safety without assertions.♻️ Optional: Introduce a non-nullable type
+interface BillingCycleInfoRequired { + subscription_anchor_start: string + subscription_anchor_end: string +} + -function getDefaultBillingCycleRange(referenceDate = new Date()): BillingCycleInfo { +function getDefaultBillingCycleRange(referenceDate = new Date()): BillingCycleInfoRequired { const start = new Date(Date.UTC(referenceDate.getUTCFullYear(), referenceDate.getUTCMonth(), 1, 0, 0, 0, 0)) const end = new Date(Date.UTC(referenceDate.getUTCFullYear(), referenceDate.getUTCMonth() + 1, 1, 0, 0, 0, 0)) return {Then type
resolvedBillingCycleasBillingCycleInfoRequired.Also applies to: 91-93
riderx
force-pushed
the
riderx/allow-credit-no-plan
branch
from
92be18c to
85f7ab0
Compare
riderx
force-pushed
the
riderx/allow-credit-no-plan
branch
from
0facd5a to
eac9d88
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@supabase/migrations/20260127121000_allow_credits_without_plan.sql`:
- Around line 271-295: The two functions disagree on how trial expiration is
compared; unify them by using the same timestamp-granular comparison (trial_at >
now()) in both functions. Update the trial check in is_paying_and_good_plan_org
(replace the trial_at::date - (now())::date > 0 expression) so it matches the
check used in is_paying_and_good_plan_org_action (si.trial_at > now()), ensuring
both functions use si.trial_at > now() for consistency.
🧹 Nitpick comments (5)
supabase/tests/11_test_plan.sql (1)
27-31: Consider adding tests for the new credit-bypass path.The UPDATE correctly exhausts credits so existing plan-action tests remain valid. However, this test file doesn't exercise the new behavior where
available_credits > 0bypasses plan checks — which is the core feature of this PR. Consider adding at least one test case that verifiesis_paying_and_good_plan_org_actionreturnstruewhen the org has available credits but no active/good plan.supabase/functions/_backend/private/credits.ts (1)
90-100: Extract duplicate fallback callback.The
fetchPlancallbacks on lines 90-98 and 117-125 are identical. Consider extracting to a local helper to reduce duplication.♻️ Proposed refactor
async function getCreditTopUpProductId(c: AppContext, customerId: string, token: string): Promise<{ productId: string }> { const supabase = supabaseClient(c, token) + const fetchSoloCreditId = async () => { + const { data, error } = await supabase + .from('plans') + .select('credit_id') + .eq('name', 'Solo') + .single() + if (error) + throw error + return data ?? null + } + const { data: stripeInfo, error: stripeInfoError } = await supabase ... if (stripeInfoError || !stripeInfo?.product_id) { ... - const productId = await getFallbackCreditProductId(c, customerId, async () => { - const { data, error } = await supabase - .from('plans') - .select('credit_id') - .eq('name', 'Solo') - .single() - if (error) - throw error - return data ?? null - }) + const productId = await getFallbackCreditProductId(c, customerId, fetchSoloCreditId) return { productId } } ... if (planError || !plan?.credit_id) { ... - const productId = await getFallbackCreditProductId(c, customerId, async () => { - const { data, error } = await supabase - .from('plans') - .select('credit_id') - .eq('name', 'Solo') - .single() - if (error) - throw error - return data ?? null - }) + const productId = await getFallbackCreditProductId(c, customerId, fetchSoloCreditId) return { productId } }Also applies to: 117-127
supabase/migrations/20260127121000_allow_credits_without_plan.sql (1)
141-149:user_meets_password_policyis called twice per org row.Lines 145-146 evaluate
public.user_meets_password_policy(userid, o.id)and thenNOT public.user_meets_password_policy(userid, o.id). Consider computing it once and deriving both values:♻️ Proposed refactor
password_policy_access AS ( SELECT o.id AS org_id, o.password_policy_config, - public.user_meets_password_policy(userid, o.id) AS password_has_access, - NOT public.user_meets_password_policy(userid, o.id) AS should_redact_password + ppa_inner.meets_policy AS password_has_access, + NOT ppa_inner.meets_policy AS should_redact_password FROM public.orgs o JOIN user_orgs uo ON uo.org_id = o.id + CROSS JOIN LATERAL ( + SELECT public.user_meets_password_policy(userid, o.id) AS meets_policy + ) ppa_inner )supabase/functions/_backend/triggers/stripe_event.ts (1)
62-69: Same duplicate fallback pattern as incredits.ts.The Solo plan fallback callback (lines 62-68 and 93-99) is duplicated within this function. Consider extracting it, similar to the suggestion for
credits.ts.♻️ Proposed refactor
async function getCreditTopUpProductIdFromCustomer(c: Context, customerId: string): Promise<string> { const pgClient = getPgClient(c, true) const drizzleClient = getDrizzleClient(pgClient) + const fetchSoloCreditId = async () => { + const [fallbackPlan] = await drizzleClient + .select({ credit_id: schema.plans.credit_id }) + .from(schema.plans) + .where(eq(schema.plans.name, 'Solo')) + .limit(1) + return fallbackPlan ?? null + } try { ... if (stripeInfoError || !stripeInfo?.product_id) { ... - return await getFallbackCreditProductId(c, customerId, async () => { - const [fallbackPlan] = await drizzleClient - .select({ credit_id: schema.plans.credit_id }) - .from(schema.plans) - .where(eq(schema.plans.name, 'Solo')) - .limit(1) - return fallbackPlan ?? null - }) + return await getFallbackCreditProductId(c, customerId, fetchSoloCreditId) } ... if (planError || !plan?.credit_id) { ... - return await getFallbackCreditProductId(c, customerId, async () => { - const [fallbackPlan] = await drizzleClient - .select({ credit_id: schema.plans.credit_id }) - .from(schema.plans) - .where(eq(schema.plans.name, 'Solo')) - .limit(1) - return fallbackPlan ?? null - }) + return await getFallbackCreditProductId(c, customerId, fetchSoloCreditId) }Also applies to: 93-100
supabase/functions/_backend/utils/plans.ts (1)
117-118: Redundant non-null assertions.
resolvedBillingCycleis typed asBillingCycleRangewhere both fields arestring(non-nullable). The!operators on lines 117-118 are unnecessary.♻️ Remove redundant assertions
- p_billing_cycle_start: resolvedBillingCycle.subscription_anchor_start!, - p_billing_cycle_end: resolvedBillingCycle.subscription_anchor_end!, + p_billing_cycle_start: resolvedBillingCycle.subscription_anchor_start, + p_billing_cycle_end: resolvedBillingCycle.subscription_anchor_end,
| CREATE OR REPLACE FUNCTION "public"."is_paying_and_good_plan_org_action"("orgid" "uuid", "actions" "public"."action_type"[]) RETURNS boolean | ||
| LANGUAGE "plpgsql" SECURITY DEFINER | ||
| SET "search_path" TO '' | ||
| AS $$ | ||
| DECLARE org_customer_id text; result boolean; has_credits boolean; | ||
| BEGIN | ||
| SELECT EXISTS ( | ||
| SELECT 1 | ||
| FROM public.usage_credit_balances ucb | ||
| WHERE ucb.org_id = orgid | ||
| AND COALESCE(ucb.available_credits, 0) > 0 | ||
| ) INTO has_credits; | ||
|
|
||
| IF has_credits THEN | ||
| RETURN true; | ||
| END IF; | ||
|
|
||
| SELECT o.customer_id INTO org_customer_id FROM public.orgs o WHERE o.id = orgid; | ||
| SELECT (si.trial_at > now()) OR (si.status = 'succeeded' AND NOT ( | ||
| (si.mau_exceeded AND 'mau' = ANY(actions)) OR (si.storage_exceeded AND 'storage' = ANY(actions)) OR | ||
| (si.bandwidth_exceeded AND 'bandwidth' = ANY(actions)) OR (si.build_time_exceeded AND 'build_time' = ANY(actions)))) | ||
| INTO result FROM public.stripe_info si WHERE si.customer_id = org_customer_id LIMIT 1; | ||
| RETURN COALESCE(result, false); | ||
| END; | ||
| $$; |
There was a problem hiding this comment.
Inconsistent trial date comparison between the two functions.
is_paying_and_good_plan_org (line 262) uses date arithmetic: trial_at::date - (now())::date > 0, which is day-granular (trial day itself is excluded). is_paying_and_good_plan_org_action (line 289) uses si.trial_at > now(), which is timestamp-granular (trial end is the exact moment). This can produce different results near midnight or on the trial expiry day itself.
Consider aligning both to the same comparison style for consistency.
Proposed fix for consistency
-- In is_paying_and_good_plan_org_action, align trial check:
- SELECT (si.trial_at > now()) OR (si.status = 'succeeded' AND NOT (
+ SELECT (si.trial_at::date > now()::date) OR (si.status = 'succeeded' AND NOT (Or alternatively update is_paying_and_good_plan_org to use trial_at > now() — just pick one style.
📝 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.
| CREATE OR REPLACE FUNCTION "public"."is_paying_and_good_plan_org_action"("orgid" "uuid", "actions" "public"."action_type"[]) RETURNS boolean | |
| LANGUAGE "plpgsql" SECURITY DEFINER | |
| SET "search_path" TO '' | |
| AS $$ | |
| DECLARE org_customer_id text; result boolean; has_credits boolean; | |
| BEGIN | |
| SELECT EXISTS ( | |
| SELECT 1 | |
| FROM public.usage_credit_balances ucb | |
| WHERE ucb.org_id = orgid | |
| AND COALESCE(ucb.available_credits, 0) > 0 | |
| ) INTO has_credits; | |
| IF has_credits THEN | |
| RETURN true; | |
| END IF; | |
| SELECT o.customer_id INTO org_customer_id FROM public.orgs o WHERE o.id = orgid; | |
| SELECT (si.trial_at > now()) OR (si.status = 'succeeded' AND NOT ( | |
| (si.mau_exceeded AND 'mau' = ANY(actions)) OR (si.storage_exceeded AND 'storage' = ANY(actions)) OR | |
| (si.bandwidth_exceeded AND 'bandwidth' = ANY(actions)) OR (si.build_time_exceeded AND 'build_time' = ANY(actions)))) | |
| INTO result FROM public.stripe_info si WHERE si.customer_id = org_customer_id LIMIT 1; | |
| RETURN COALESCE(result, false); | |
| END; | |
| $$; | |
| CREATE OR REPLACE FUNCTION "public"."is_paying_and_good_plan_org_action"("orgid" "uuid", "actions" "public"."action_type"[]) RETURNS boolean | |
| LANGUAGE "plpgsql" SECURITY DEFINER | |
| SET "search_path" TO '' | |
| AS $$ | |
| DECLARE org_customer_id text; result boolean; has_credits boolean; | |
| BEGIN | |
| SELECT EXISTS ( | |
| SELECT 1 | |
| FROM public.usage_credit_balances ucb | |
| WHERE ucb.org_id = orgid | |
| AND COALESCE(ucb.available_credits, 0) > 0 | |
| ) INTO has_credits; | |
| IF has_credits THEN | |
| RETURN true; | |
| END IF; | |
| SELECT o.customer_id INTO org_customer_id FROM public.orgs o WHERE o.id = orgid; | |
| SELECT (si.trial_at::date > now()::date) OR (si.status = 'succeeded' AND NOT ( | |
| (si.mau_exceeded AND 'mau' = ANY(actions)) OR (si.storage_exceeded AND 'storage' = ANY(actions)) OR | |
| (si.bandwidth_exceeded AND 'bandwidth' = ANY(actions)) OR (si.build_time_exceeded AND 'build_time' = ANY(actions)))) | |
| INTO result FROM public.stripe_info si WHERE si.customer_id = org_customer_id LIMIT 1; | |
| RETURN COALESCE(result, false); | |
| END; | |
| $$; |
🤖 Prompt for AI Agents
In `@supabase/migrations/20260127121000_allow_credits_without_plan.sql` around
lines 271 - 295, The two functions disagree on how trial expiration is compared;
unify them by using the same timestamp-granular comparison (trial_at > now()) in
both functions. Update the trial check in is_paying_and_good_plan_org (replace
the trial_at::date - (now())::date > 0 expression) so it matches the check used
in is_paying_and_good_plan_org_action (si.trial_at > now()), ensuring both
functions use si.trial_at > now() for consistency.
Reduce edge runtime CPU usage in CI by short-circuiting org member notification code when Bento env vars are placeholders or missing, and ensure audit logs tests add org membership.
CI uses a placeholder STRIPE_SECRET_KEY (e.g. 'test'); cron_stat_org must still compute org plan fit/exceeded flags and update stripe_info.is_good_plan without performing Stripe API calls. Remove the unnecessary Stripe-key gate from checkPlanStatusOnly.
Plan usage/fit and is_good_plan checks are Postgres RPCs and should still run when STRIPE_SECRET_KEY is set to placeholders like 'test' in CI. Only skip the RPCs when STRIPE_SECRET_KEY is truly unset.
Reduce edge runtime CPU usage in CI by short-circuiting org member notification code when Bento env vars are placeholders or missing, and ensure audit logs tests add org membership.
|
Summary (AI generated)
Test plan (AI generated)
Screenshots (AI generated)
Not applicable (backend-only change).
Checklist (AI generated)
.
accordingly.
my tests
Generated with AI
Summary by CodeRabbit
New Features
Improvements
Bug Fixes / Behavior
Tests