From d333f3f6e601b9dd85d3c62f0ef747289f045c5d Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 3 Nov 2026 17:34:49 +0100 Subject: [PATCH] fix(stripe): end paid subscriptions that lost their Stripe subscription ID Webhooks matched rows only by subscription ID, so a paid row without one stayed active after the customer canceled. The deleted and updated events now verify such a row, the verify job also finds canceled subscriptions, and the invoice-failed job counts only invoices of the row's subscription. cloud:sync-stripe-subscriptions reports these rows, and cloud:fix-subscription --verify-all writes its CSV to a file and shows details only with -v. --- CHANGELOG.md | 1 + .../Stripe/SyncStripeSubscriptions.php | 78 ++++++++----- .../Commands/Cloud/CloudFixSubscription.php | 104 +++++++++++------- .../Cloud/SyncStripeSubscriptions.php | 3 +- app/Jobs/StripeProcessJob.php | 39 +++++-- app/Jobs/SubscriptionInvoiceFailedJob.php | 26 +++-- .../VerifyStripeSubscriptionStatusJob.php | 14 ++- .../Subscription/StripeProcessJobTest.php | 56 ++++++++++ .../SubscriptionInvoiceFailedJobTest.php | 66 +++++++++++ .../SyncStripeSubscriptionsActionTest.php | 42 +++++++ .../VerifyStripeSubscriptionStatusJobTest.php | 24 ++++ 11 files changed, 360 insertions(+), 93 deletions(-) create mode 100644 tests/Feature/Subscription/SubscriptionInvoiceFailedJobTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 4883ed923a..2df7cc9752 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), - Save deployments started through the MCP server as MCP deployments, not as API deployments. The deployment list shows them with the **MCP** source and filter, and the API and MCP deployment data have a new `is_mcp` field. The deployments to additional servers keep the source of the main deployment, and the audit log records actions that MCP tools do through the API as MCP events - Save the update channel in **Settings > Updates** when you select **Preview** or **Nightly**. Before, the "Save changes" bar did not open for these channels, so the change was lost when you refreshed the page - Save the healthcheck **Check type** when it is the only change. Before, the "Save changes" bar did not open when you changed only the check type, for example from **Container command** back to **HTTP request** +- Coolify Cloud: end a paid subscription that lost its link to Stripe when the customer cancels it. Before, it stayed active, and `cloud:sync-stripe-subscriptions` did not report it ## [v4.6.0] - 2026-11-02 diff --git a/app/Actions/Stripe/SyncStripeSubscriptions.php b/app/Actions/Stripe/SyncStripeSubscriptions.php index 1e430bbccd..443f9e884f 100644 --- a/app/Actions/Stripe/SyncStripeSubscriptions.php +++ b/app/Actions/Stripe/SyncStripeSubscriptions.php @@ -18,16 +18,16 @@ class SyncStripeSubscriptions return ['error' => 'Not running on Cloud or Stripe not configured']; } - $subscriptions = Subscription::whereNotNull('stripe_subscription_id') - ->where('stripe_invoice_paid', true) - ->get(); + // Paid rows without a subscription ID are included: no webhook can end them. + $subscriptions = Subscription::where('stripe_invoice_paid', true)->get(); $stripe = app()->bound(StripeClient::class) ? app(StripeClient::class) : new StripeClient(config('subscription.stripe_api_key')); - // Bulk fetch all valid subscription IDs from Stripe (active + past_due) - $validStripeIds = $this->fetchValidStripeSubscriptionIds($stripe, $onProgress); + // Bulk fetch all valid subscriptions from Stripe (active + past_due) + $validStripeCustomers = $this->fetchValidStripeSubscriptions($stripe, $onProgress); + $validStripeIds = array_keys($validStripeCustomers); // Find DB subscriptions not in the valid set $staleSubscriptions = $subscriptions->filter( @@ -47,27 +47,51 @@ class SyncStripeSubscriptions foreach ($staleSubscriptions->values() as $index => $subscription) { $onProgress?->__invoke('checking', $index + 1, $staleSubscriptionCount); - try { - $stripeSubscription = $stripe->subscriptions->retrieve( - $subscription->stripe_subscription_id - ); - $stripeStatus = $stripeSubscription->status; + if (! $subscription->stripe_subscription_id) { + $customerId = $subscription->stripe_customer_id; + $stripeStatus = 'missing'; + $liveSubscriptionId = $customerId ? array_search($customerId, $validStripeCustomers, true) : false; - usleep(100000); // 100ms rate limit delay - } catch (\Exception $e) { - $errors[] = [ - 'subscription_id' => $subscription->id, - 'error' => $e->getMessage(), - ]; + if ($liveSubscriptionId !== false && ! Subscription::where('stripe_subscription_id', $liveSubscriptionId)->exists()) { + $discrepancies[] = [ + 'subscription_id' => $subscription->id, + 'team_id' => $subscription->team_id, + 'stripe_subscription_id' => $liveSubscriptionId, + 'stripe_status' => $stripeStatus, + 'resolution' => 'link_subscription', + ]; - continue; + if ($fix) { + $subscription->update(['stripe_subscription_id' => $liveSubscriptionId]); + $fixedCount++; + } + + continue; + } + } else { + try { + $stripeSubscription = $stripe->subscriptions->retrieve( + $subscription->stripe_subscription_id + ); + $stripeStatus = $stripeSubscription->status; + $customerId = $stripeSubscription->customer; + + usleep(100000); // 100ms rate limit delay + } catch (\Exception $e) { + $errors[] = [ + 'subscription_id' => $subscription->id, + 'error' => $e->getMessage(), + ]; + + continue; + } + + if (in_array($stripeStatus, self::VALID_STRIPE_STATUSES, true)) { + continue; + } } - if (in_array($stripeStatus, self::VALID_STRIPE_STATUSES, true)) { - continue; - } - - $activeSub = $this->findActiveSubscriptionByEmail($stripe, $stripeSubscription->customer); + $activeSub = $customerId ? $this->findActiveSubscriptionByEmail($stripe, $customerId) : null; $validReplacement = Subscription::query() ->where('team_id', $subscription->team_id) ->where('id', '!=', $subscription->id) @@ -81,7 +105,7 @@ class SyncStripeSubscriptions 'team_id' => $subscription->team_id, 'email' => $activeSub['email'], 'old_stripe_subscription_id' => $subscription->stripe_subscription_id, - 'old_stripe_customer_id' => $stripeSubscription->customer, + 'old_stripe_customer_id' => $customerId, 'new_stripe_subscription_id' => $activeSub['subscription_id'], 'new_stripe_customer_id' => $activeSub['customer_id'], 'new_status' => $activeSub['status'], @@ -220,18 +244,18 @@ class SyncStripeSubscriptions } /** - * Bulk fetch all active and past_due subscription IDs from Stripe. + * Bulk fetch all active and past_due subscriptions from Stripe. * - * @return array + * @return array Stripe customer ID by subscription ID */ - private function fetchValidStripeSubscriptionIds(StripeClient $stripe, ?\Closure $onProgress = null): array + private function fetchValidStripeSubscriptions(StripeClient $stripe, ?\Closure $onProgress = null): array { $validIds = []; $fetched = 0; foreach (self::VALID_STRIPE_STATUSES as $status) { foreach ($stripe->subscriptions->all(['status' => $status, 'limit' => 100])->autoPagingIterator() as $sub) { - $validIds[] = $sub->id; + $validIds[$sub->id] = $sub->customer ?? null; $fetched++; if ($onProgress) { diff --git a/app/Console/Commands/Cloud/CloudFixSubscription.php b/app/Console/Commands/Cloud/CloudFixSubscription.php index 65a4fa4986..7c0303f9d0 100644 --- a/app/Console/Commands/Cloud/CloudFixSubscription.php +++ b/app/Console/Commands/Cloud/CloudFixSubscription.php @@ -19,7 +19,8 @@ class CloudFixSubscription extends Command {--verify-all : Verify all active subscriptions against Stripe} {--fix-verified : Fix discrepancies found during verification} {--dry-run : Show what would be fixed without making changes} - {--one : Only fix the first found subscription}'; + {--one : Only fix the first found subscription} + {--csv= : Path of the CSV file that --verify-all writes (default: storage/app/subscription-verification-.csv)}'; /** * The console command description. @@ -348,7 +349,16 @@ class CloudFixSubscription extends Command $this->info("Found {$totalCount} teams with active subscriptions in database"); $this->newLine(); - $out = fopen('php://output', 'w'); + $csvPath = $this->option('csv') ?: storage_path('app/subscription-verification-'.now()->format('Y-m-d-His').'.csv'); + $out = fopen($csvPath, 'w'); + if ($out === false) { + $this->error("Cannot write the CSV file: {$csvPath}"); + + return 1; + } + + // Details for each team only with -v; otherwise a progress bar. + $progressBar = $this->output->isVerbose() ? null : $this->output->createProgressBar($totalCount); // CSV header fputcsv($out, [ @@ -391,20 +401,20 @@ class CloudFixSubscription extends Command $action = 'none'; if (! $subscription->stripe_subscription_id) { - $this->line("Team {$team->id}: Missing subscription ID, searching in Stripe..."); + $this->detail("Team {$team->id}: Missing subscription ID, searching in Stripe..."); $foundResult = null; $searchMethod = null; // Search by customer ID if ($subscription->stripe_customer_id) { - $this->line(" → Searching by customer ID: {$subscription->stripe_customer_id}"); + $this->detail(" → Searching by customer ID: {$subscription->stripe_customer_id}"); $foundResult = $this->searchSubscriptionsByCustomer($stripe, $subscription->stripe_customer_id); if ($foundResult) { $searchMethod = $foundResult['method']; } } else { - $this->line(' → No customer ID available'); + $this->detail(' → No customer ID available'); } // Search by emails if not found @@ -416,10 +426,10 @@ class CloudFixSubscription extends Command // Update customer ID if different if (isset($foundResult['customer_id']) && $subscription->stripe_customer_id !== $foundResult['customer_id']) { if ($isDryRun) { - $this->warn(" ⚠ Would update customer ID from {$subscription->stripe_customer_id} to {$foundResult['customer_id']}"); + $this->detail(" ⚠ Would update customer ID from {$subscription->stripe_customer_id} to {$foundResult['customer_id']}"); } elseif ($shouldFix) { $subscription->update(['stripe_customer_id' => $foundResult['customer_id']]); - $this->info(" ✓ Updated customer ID to {$foundResult['customer_id']}"); + $this->detail(" ✓ Updated customer ID to {$foundResult['customer_id']}"); } } } @@ -453,7 +463,7 @@ class CloudFixSubscription extends Command ]); } else { // Found subscription but it's canceled/expired - needs to be deactivated - $this->warn(" → Found {$foundResult['status']} subscription {$foundResult['subscription']->id} - needs deactivation"); + $this->detail(" → Found {$foundResult['status']} subscription {$foundResult['subscription']->id} - needs deactivation"); $result = $this->handleMissingSubscription($team, $subscription, $foundResult['status'], $isDryRun, $shouldFix, $stats); @@ -472,7 +482,7 @@ class CloudFixSubscription extends Command } } else { // No subscription found at all - $this->line(' → No subscription found'); + $this->detail(' → No subscription found'); $stripeStatus = 'not_found'; $result = $this->handleMissingSubscription($team, $subscription, $stripeStatus, $isDryRun, $shouldFix, $stats); @@ -493,7 +503,7 @@ class CloudFixSubscription extends Command } else { // First validate the subscription ID format if (! str_starts_with($subscription->stripe_subscription_id, 'sub_')) { - $this->warn(" ⚠ Invalid subscription ID format (doesn't start with 'sub_')"); + $this->detail(" ⚠ Invalid subscription ID format (doesn't start with 'sub_')"); } try { @@ -516,7 +526,7 @@ class CloudFixSubscription extends Command // Ensure past_due flag is set if (! $subscription->stripe_past_due) { if ($isDryRun) { - $this->info("Would set stripe_past_due=true for Team {$team->id}"); + $this->detail("Would set stripe_past_due=true for Team {$team->id}"); } elseif ($shouldFix) { $subscription->update(['stripe_past_due' => true]); } @@ -545,7 +555,7 @@ class CloudFixSubscription extends Command ]); if ($isDryRun) { - $this->info("Would deactivate subscription for Team {$team->id} - status: {$stripeStatus}"); + $this->detail("Would deactivate subscription for Team {$team->id} - status: {$stripeStatus}"); } elseif ($shouldFix) { $this->fixSubscription($team, $subscription, $stripeStatus); $stats['fixed']++; @@ -573,11 +583,11 @@ class CloudFixSubscription extends Command } } catch (InvalidRequestException $e) { - $this->error(' → Error: '.$e->getMessage()); + $this->detail(' → Error: '.$e->getMessage()); if ($e->getStripeCode() === 'resource_missing' || $e->getHttpStatus() === 404) { // Subscription doesn't exist, try to find by customer ID - $this->warn(" → Subscription not found, checking customer's subscriptions..."); + $this->detail(" → Subscription not found, checking customer's subscriptions..."); $foundResult = null; if ($subscription->stripe_customer_id) { @@ -586,7 +596,7 @@ class CloudFixSubscription extends Command if ($foundResult && isset($foundResult['subscription']) && in_array($foundResult['status'], ['active', 'past_due'])) { // Found an active subscription with different ID - $this->warn(" → ID mismatch! DB: {$subscription->stripe_subscription_id}, Stripe: {$foundResult['subscription']->id}"); + $this->detail(" → ID mismatch! DB: {$subscription->stripe_subscription_id}, Stripe: {$foundResult['subscription']->id}"); fputcsv($out, [ $team->id, @@ -602,7 +612,7 @@ class CloudFixSubscription extends Command ]); if ($isDryRun) { - $this->warn(" → Would update subscription ID to {$foundResult['subscription']->id}"); + $this->detail(" → Would update subscription ID to {$foundResult['subscription']->id}"); } elseif ($shouldFix) { $subscription->update([ 'stripe_subscription_id' => $foundResult['subscription']->id, @@ -610,7 +620,7 @@ class CloudFixSubscription extends Command 'stripe_past_due' => $foundResult['status'] === 'past_due', ]); $stats['fixed']++; - $this->info(' → Updated subscription ID'); + $this->detail(' → Updated subscription ID'); } $stats[$foundResult['status'] === 'active' ? 'valid_active' : 'valid_past_due']++; @@ -635,7 +645,7 @@ class CloudFixSubscription extends Command } else { // Other API error $stats['errors']++; - $this->error(' → API Error - not marking as deleted'); + $this->detail(' → API Error - not marking as deleted'); fputcsv($out, [ $team->id, @@ -651,7 +661,7 @@ class CloudFixSubscription extends Command ]); } } catch (\Exception $e) { - $this->error(' → Unexpected error: '.$e->getMessage()); + $this->detail(' → Unexpected error: '.$e->getMessage()); $stats['errors']++; fputcsv($out, [ @@ -670,11 +680,13 @@ class CloudFixSubscription extends Command } $processedCount++; - if ($processedCount % 100 === 0) { - $this->info("Processed {$processedCount}/{$totalCount} subscriptions..."); + $progressBar?->advance(); + if (! $progressBar && $processedCount % 100 === 0) { + $this->detail("Processed {$processedCount}/{$totalCount} subscriptions..."); } } + $progressBar?->finish(); fclose($out); // Print summary @@ -710,9 +722,25 @@ class CloudFixSubscription extends Command $this->comment('Run with --fix-verified to fix the discrepancies'); } + $this->newLine(); + $this->info("Problem rows written to: {$csvPath}"); + if (! $this->output->isVerbose()) { + $this->comment('Run with -v to see the details for each team.'); + } + return 0; } + /** + * Print a detail line of the verification only in verbose mode (-v). + */ + private function detail(string $message): void + { + if ($this->output->isVerbose()) { + $this->line($message); + } + } + /** * Fix a subscription based on its status */ @@ -741,13 +769,13 @@ class CloudFixSubscription extends Command 'status' => 'all', ]); - $this->line(' → Found '.count($subscriptions->data).' subscription(s) for customer'); + $this->detail(' → Found '.count($subscriptions->data).' subscription(s) for customer'); // Look for active/past_due first foreach ($subscriptions->data as $sub) { - $this->line(" - Subscription {$sub->id}: status={$sub->status}"); + $this->detail(" - Subscription {$sub->id}: status={$sub->status}"); if (in_array($sub->status, ['active', 'past_due'])) { - $this->info(" ✓ Found active/past_due subscription: {$sub->id}"); + $this->detail(" ✓ Found active/past_due subscription: {$sub->id}"); return ['subscription' => $sub, 'status' => $sub->status, 'method' => 'customer_id']; } @@ -756,14 +784,14 @@ class CloudFixSubscription extends Command // If not requiring active and there are subscriptions, return first one if (! $requireActive && count($subscriptions->data) > 0) { $sub = $subscriptions->data[0]; - $this->warn(" ⚠ Only found {$sub->status} subscription: {$sub->id}"); + $this->detail(" ⚠ Only found {$sub->status} subscription: {$sub->id}"); return ['subscription' => $sub, 'status' => $sub->status, 'method' => 'customer_id_first']; } return null; } catch (\Exception $e) { - $this->error(' → Error searching by customer ID: '.$e->getMessage()); + $this->detail(' → Error searching by customer ID: '.$e->getMessage()); return null; } @@ -774,10 +802,10 @@ class CloudFixSubscription extends Command */ private function searchSubscriptionsByEmails(StripeClient $stripe, $emails) { - $this->line(' → Searching by team member emails...'); + $this->detail(' → Searching by team member emails...'); foreach ($emails as $email) { - $this->line(" → Checking email: {$email}"); + $this->detail(" → Checking email: {$email}"); try { $customers = $stripe->customers->all([ @@ -786,15 +814,15 @@ class CloudFixSubscription extends Command ]); if (count($customers->data) === 0) { - $this->line(' - No customers found'); + $this->detail(' - No customers found'); continue; } - $this->line(' - Found '.count($customers->data).' customer(s)'); + $this->detail(' - Found '.count($customers->data).' customer(s)'); foreach ($customers->data as $customer) { - $this->line(" - Checking customer {$customer->id}"); + $this->detail(" - Checking customer {$customer->id}"); $result = $this->searchSubscriptionsByCustomer($stripe, $customer->id, true); if ($result) { @@ -805,7 +833,7 @@ class CloudFixSubscription extends Command } } } catch (\Exception $e) { - $this->error(" - Error searching for email {$email}: ".$e->getMessage()); + $this->detail(" - Error searching for email {$email}: ".$e->getMessage()); } } @@ -818,11 +846,11 @@ class CloudFixSubscription extends Command private function handleFoundSubscription($team, $subscription, $foundSub, $searchMethod, $isDryRun, $shouldFix, &$stats) { $stripeStatus = $foundSub->status; - $this->info(" ✓ FOUND active/past_due subscription {$foundSub->id} (status: {$stripeStatus})"); + $this->detail(" ✓ FOUND active/past_due subscription {$foundSub->id} (status: {$stripeStatus})"); // Only update if it's active or past_due if (! in_array($stripeStatus, ['active', 'past_due'])) { - $this->error(" ERROR: handleFoundSubscription called with {$stripeStatus} subscription!"); + $this->detail(" ERROR: handleFoundSubscription called with {$stripeStatus} subscription!"); return [ 'id' => $foundSub->id, @@ -833,7 +861,7 @@ class CloudFixSubscription extends Command } if ($isDryRun) { - $this->warn(" → Would update subscription ID to {$foundSub->id} (status: {$stripeStatus})"); + $this->detail(" → Would update subscription ID to {$foundSub->id} (status: {$stripeStatus})"); } elseif ($shouldFix) { $subscription->update([ 'stripe_subscription_id' => $foundSub->id, @@ -841,7 +869,7 @@ class CloudFixSubscription extends Command 'stripe_past_due' => $stripeStatus === 'past_due', ]); $stats['fixed']++; - $this->info(" → Updated subscription ID to {$foundSub->id}"); + $this->detail(" → Updated subscription ID to {$foundSub->id}"); } // Update stats @@ -864,11 +892,11 @@ class CloudFixSubscription extends Command if ($isDryRun) { $statusMsg = $status !== 'not_found' ? "status: {$status}" : 'no subscription found in Stripe'; - $this->warn(" → Would deactivate subscription - {$statusMsg}"); + $this->detail(" → Would deactivate subscription - {$statusMsg}"); } elseif ($shouldFix) { $this->fixSubscription($team, $subscription, $status); $stats['fixed']++; - $this->info(' → Deactivated subscription'); + $this->detail(' → Deactivated subscription'); } return [ diff --git a/app/Console/Commands/Cloud/SyncStripeSubscriptions.php b/app/Console/Commands/Cloud/SyncStripeSubscriptions.php index cedacfbeb5..fa2cb0915d 100644 --- a/app/Console/Commands/Cloud/SyncStripeSubscriptions.php +++ b/app/Console/Commands/Cloud/SyncStripeSubscriptions.php @@ -65,10 +65,11 @@ class SyncStripeSubscriptions extends Command foreach ($result['discrepancies'] as $discrepancy) { $this->line(" - Subscription ID: {$discrepancy['subscription_id']}"); $this->line(" Team ID: {$discrepancy['team_id']}"); - $this->line(" Stripe ID: {$discrepancy['stripe_subscription_id']}"); + $this->line(' Stripe ID: '.($discrepancy['stripe_subscription_id'] ?? 'none')); $this->line(" Stripe Status: {$discrepancy['stripe_status']}"); $resolution = match ($discrepancy['resolution']) { 'delete_stale' => 'Delete stale local row', + 'link_subscription' => 'Link the active Stripe subscription', 'manual_review' => 'Manual review required', default => 'End local subscription', }; diff --git a/app/Jobs/StripeProcessJob.php b/app/Jobs/StripeProcessJob.php index d0d11cee65..d355d05b96 100644 --- a/app/Jobs/StripeProcessJob.php +++ b/app/Jobs/StripeProcessJob.php @@ -327,6 +327,9 @@ class StripeProcessJob implements ShouldBeEncrypted, ShouldQueue 'stripe_plan_id' => $planId, 'stripe_cancel_at_period_end' => $cancelAtPeriodEnd, ]); + if (! $subscription->stripe_subscription_id && $subscription->stripe_invoice_paid) { + VerifyStripeSubscriptionStatusJob::dispatch($subscription); + } if ($status === 'paused' || $status === 'incomplete_expired') { if ($subscription->stripe_subscription_id === $subscriptionId) { $subscription->update([ @@ -377,18 +380,15 @@ class StripeProcessJob implements ShouldBeEncrypted, ShouldQueue $customerId = data_get($data, 'customer'); $subscriptionId = data_get($data, 'id'); $subscription = Subscription::where('stripe_customer_id', $customerId)->where('stripe_subscription_id', $subscriptionId)->first(); - if ($subscription) { - $team = data_get($subscription, 'team'); - if ($team) { - $team->subscriptionEnded($subscription); - } else { - // send_internal_notification('Subscription deleted but no team found in Coolify for customer: '.$customerId); - throw new \RuntimeException("No team found in Coolify for customer: {$customerId}"); - } - } else { - // send_internal_notification('Subscription deleted but no subscription found in Coolify for customer: '.$customerId); + if (! $subscription) { + $this->verifyUnlinkedPaidSubscription($customerId); break; } + $team = data_get($subscription, 'team'); + if (! $team) { + throw new \RuntimeException("No team found in Coolify for customer: {$customerId}"); + } + $team->subscriptionEnded($subscription); break; default: throw new \RuntimeException("Unhandled event type: {$type}"); @@ -397,4 +397,23 @@ class StripeProcessJob implements ShouldBeEncrypted, ShouldQueue send_internal_notification('StripeProcessJob error: '.$e->getMessage()); } } + + /** + * A paid row without a Stripe subscription ID matches no webhook by ID. Verify it against Stripe. + */ + private function verifyUnlinkedPaidSubscription(?string $customerId): void + { + if (! $customerId) { + return; + } + + $subscription = Subscription::where('stripe_customer_id', $customerId) + ->whereNull('stripe_subscription_id') + ->where('stripe_invoice_paid', true) + ->first(); + + if ($subscription) { + VerifyStripeSubscriptionStatusJob::dispatch($subscription); + } + } } diff --git a/app/Jobs/SubscriptionInvoiceFailedJob.php b/app/Jobs/SubscriptionInvoiceFailedJob.php index 99b3f8be96..66580e4390 100755 --- a/app/Jobs/SubscriptionInvoiceFailedJob.php +++ b/app/Jobs/SubscriptionInvoiceFailedJob.php @@ -43,21 +43,23 @@ class SubscriptionInvoiceFailedJob implements ShouldBeEncrypted, ShouldQueue return; } - } - $invoices = $stripe->invoices->all([ - 'customer' => $subscription->stripe_customer_id, - 'limit' => 3, - ]); + // Only an invoice of this subscription proves payment, not one of an older subscription of the customer. + $invoices = $stripe->invoices->all([ + 'customer' => $subscription->stripe_customer_id, + 'subscription' => $subscription->stripe_subscription_id, + 'limit' => 3, + ]); - foreach ($invoices->data as $invoice) { - if ($invoice->paid && $invoice->created > (time() - 3600)) { - $subscription->update([ - 'stripe_invoice_paid' => true, - 'stripe_past_due' => false, - ]); + foreach ($invoices->data as $invoice) { + if ($invoice->paid && $invoice->created > (time() - 3600)) { + $subscription->update([ + 'stripe_invoice_paid' => true, + 'stripe_past_due' => false, + ]); - return; + return; + } } } } catch (\Exception $e) { diff --git a/app/Jobs/VerifyStripeSubscriptionStatusJob.php b/app/Jobs/VerifyStripeSubscriptionStatusJob.php index bf5e7ad3d9..942952cde1 100644 --- a/app/Jobs/VerifyStripeSubscriptionStatusJob.php +++ b/app/Jobs/VerifyStripeSubscriptionStatusJob.php @@ -31,14 +31,18 @@ class VerifyStripeSubscriptionStatusJob implements ShouldBeEncrypted, ShouldQueu $this->subscription->stripe_customer_id) { try { $stripe = app(StripeClient::class); - $subscriptions = $stripe->subscriptions->all([ + $subscriptions = collect($stripe->subscriptions->all([ 'customer' => $this->subscription->stripe_customer_id, - 'limit' => 1, - ]); + 'status' => 'all', + 'limit' => 10, + ])->data); - if ($subscriptions->data) { + $found = $subscriptions->first(fn ($sub) => in_array($sub->status, ['active', 'past_due'], true)) + ?? $subscriptions->first(); + + if ($found) { $this->subscription->update([ - 'stripe_subscription_id' => $subscriptions->data[0]->id, + 'stripe_subscription_id' => $found->id, ]); } } catch (\Exception $e) { diff --git a/tests/Feature/Subscription/StripeProcessJobTest.php b/tests/Feature/Subscription/StripeProcessJobTest.php index 02c12439dc..e92ceaa602 100644 --- a/tests/Feature/Subscription/StripeProcessJobTest.php +++ b/tests/Feature/Subscription/StripeProcessJobTest.php @@ -577,3 +577,59 @@ describe('checkout.session.expired', function () { expect($this->team->subscription()->first()->stripe_checkout_reminder_sent_at)->toBeNull(); }); }); + +describe('paid rows without a Stripe subscription ID', function () { + beforeEach(function () { + $this->unlinked = Subscription::create([ + 'team_id' => $this->team->id, + 'stripe_customer_id' => 'cus_unlinked', + 'stripe_subscription_id' => null, + 'stripe_invoice_paid' => true, + ]); + + $this->stripeSubscriptions = Mockery::mock(SubscriptionService::class); + $stripe = Mockery::mock(StripeClient::class); + $stripe->subscriptions = $this->stripeSubscriptions; + $this->instance(StripeClient::class, $stripe); + }); + + test('a deleted event of the customer ends the row', function () { + $this->stripeSubscriptions->shouldReceive('all')->andReturn((object) ['data' => [ + (object) ['id' => 'sub_canceled', 'status' => 'canceled'], + ]]); + $this->stripeSubscriptions->shouldReceive('retrieve')->with('sub_canceled')->andReturn((object) [ + 'status' => 'canceled', + 'cancel_at_period_end' => false, + ]); + + (new StripeProcessJob(['type' => 'customer.subscription.deleted', 'data' => ['object' => [ + 'customer' => 'cus_unlinked', + 'id' => 'sub_canceled', + ]]]))->handle(); + + expect($this->unlinked->fresh()->stripe_invoice_paid)->toBeFalsy() + ->and($this->unlinked->fresh()->stripe_subscription_id)->toBeNull(); + }); + + test('an updated event of the customer links the active subscription', function () { + $this->stripeSubscriptions->shouldReceive('all')->andReturn((object) ['data' => [ + (object) ['id' => 'sub_old', 'status' => 'canceled'], + (object) ['id' => 'sub_live', 'status' => 'active'], + ]]); + $this->stripeSubscriptions->shouldReceive('retrieve')->with('sub_live')->andReturn((object) [ + 'status' => 'active', + 'cancel_at_period_end' => false, + ]); + + (new StripeProcessJob(['type' => 'customer.subscription.updated', 'data' => ['object' => [ + 'id' => 'sub_live', + 'customer' => 'cus_unlinked', + 'status' => 'active', + 'cancel_at_period_end' => false, + 'items' => ['data' => [['plan' => ['id' => 'price_monthly'], 'price' => ['lookup_key' => 'monthly']]]], + ]]]))->handle(); + + expect($this->unlinked->fresh()->stripe_subscription_id)->toBe('sub_live') + ->and($this->unlinked->fresh()->stripe_invoice_paid)->toBeTruthy(); + }); +}); diff --git a/tests/Feature/Subscription/SubscriptionInvoiceFailedJobTest.php b/tests/Feature/Subscription/SubscriptionInvoiceFailedJobTest.php new file mode 100644 index 0000000000..ea4f215914 --- /dev/null +++ b/tests/Feature/Subscription/SubscriptionInvoiceFailedJobTest.php @@ -0,0 +1,66 @@ +set('constants.coolify.self_hosted', false); + config()->set('subscription.provider', 'stripe'); + config()->set('subscription.stripe_api_key', 'sk_test_fake'); + + // The failure email builds a portal session with the static Stripe API. Make it fail at once, without network. + $this->apiBase = Stripe::$apiBase; + Stripe::$apiBase = 'http://127.0.0.1:9'; + + $this->team = Team::factory()->create(); + $this->subscription = Subscription::create([ + 'team_id' => $this->team->id, + 'stripe_customer_id' => 'cus_failed', + 'stripe_subscription_id' => 'sub_new', + 'stripe_invoice_paid' => false, + ]); + + $this->invoices = Mockery::mock(InvoiceService::class); + $this->subscriptions = Mockery::mock(SubscriptionService::class); + $stripe = Mockery::mock(StripeClient::class); + $stripe->invoices = $this->invoices; + $stripe->subscriptions = $this->subscriptions; + $this->instance(StripeClient::class, $stripe); + + $this->subscriptions->shouldReceive('retrieve')->with('sub_new')->andReturn((object) ['status' => 'incomplete']); + + // A recent paid invoice of an older subscription of the same customer. + $this->invoices->shouldReceive('all')->andReturnUsing(fn (array $parameters) => (object) [ + 'data' => ($parameters['subscription'] ?? null) === null + ? [(object) ['paid' => true, 'created' => time(), 'subscription' => 'sub_old']] + : [], + ]); +}); + +afterEach(function () { + Stripe::$apiBase = $this->apiBase; +}); + +test('a paid invoice of another subscription does not mark the row as paid', function () { + // The job goes on to the failure email, which needs the Stripe portal. + expect(fn () => (new SubscriptionInvoiceFailedJob($this->team))->handle())->toThrow(ApiConnectionException::class); + + expect($this->subscription->fresh()->stripe_invoice_paid)->toBeFalsy(); +}); + +test('a row without a subscription ID is not marked as paid by customer invoices', function () { + $this->subscription->update(['stripe_subscription_id' => null]); + + expect(fn () => (new SubscriptionInvoiceFailedJob($this->team->fresh()))->handle())->toThrow(ApiConnectionException::class); + + expect($this->subscription->fresh()->stripe_invoice_paid)->toBeFalsy(); +}); diff --git a/tests/Feature/Subscription/SyncStripeSubscriptionsActionTest.php b/tests/Feature/Subscription/SyncStripeSubscriptionsActionTest.php index a59ae659de..922e120a7a 100644 --- a/tests/Feature/Subscription/SyncStripeSubscriptionsActionTest.php +++ b/tests/Feature/Subscription/SyncStripeSubscriptionsActionTest.php @@ -293,3 +293,45 @@ test('the terminal command passes the fix option to the reconciliation action', ->expectsOutput('Skipped for manual review: 1') ->assertSuccessful(); }); + +test('a paid row without a subscription ID is ended when the customer has no live Stripe subscription', function () { + $this->subscription->update(['stripe_subscription_id' => null]); + + $stripe = Mockery::mock(StripeClient::class); + $subscriptions = Mockery::mock(SubscriptionService::class); + $customers = Mockery::mock(CustomerService::class); + $stripe->subscriptions = $subscriptions; + $stripe->customers = $customers; + + $subscriptions->shouldReceive('all')->twice()->andReturn(stripeSubscriptionCollection()); + $customers->shouldReceive('retrieve')->with('cus_stale')->andReturn((object) ['email' => null]); + + $this->instance(StripeClient::class, $stripe); + + $result = SyncStripeSubscriptions::run(fix: true); + + expect($result['discrepancies'])->toHaveCount(1) + ->and($result['discrepancies'][0]['resolution'])->toBe('end_subscription') + ->and($this->subscription->fresh()->stripe_invoice_paid)->toBeFalsy(); +}); + +test('a paid row without a subscription ID is linked to the live Stripe subscription of its customer', function () { + $this->subscription->update(['stripe_subscription_id' => null]); + + $stripe = Mockery::mock(StripeClient::class); + $subscriptions = Mockery::mock(SubscriptionService::class); + $stripe->subscriptions = $subscriptions; + + $subscriptions->shouldReceive('all')->with(['status' => 'active', 'limit' => 100]) + ->andReturn(stripeSubscriptionCollection([['id' => 'sub_live', 'customer' => 'cus_stale']])); + $subscriptions->shouldReceive('all')->with(['status' => 'past_due', 'limit' => 100]) + ->andReturn(stripeSubscriptionCollection()); + + $this->instance(StripeClient::class, $stripe); + + $result = SyncStripeSubscriptions::run(fix: true); + + expect($result['discrepancies'][0]['resolution'])->toBe('link_subscription') + ->and($this->subscription->fresh()->stripe_subscription_id)->toBe('sub_live') + ->and($this->subscription->fresh()->stripe_invoice_paid)->toBeTruthy(); +}); diff --git a/tests/Feature/Subscription/VerifyStripeSubscriptionStatusJobTest.php b/tests/Feature/Subscription/VerifyStripeSubscriptionStatusJobTest.php index be8661b6c5..6a5a3cc88c 100644 --- a/tests/Feature/Subscription/VerifyStripeSubscriptionStatusJobTest.php +++ b/tests/Feature/Subscription/VerifyStripeSubscriptionStatusJobTest.php @@ -100,3 +100,27 @@ test('subscriptionEnded is called for canceled status', function () { expect($this->subscription->stripe_invoice_paid)->toBeFalsy(); expect($this->subscription->stripe_subscription_id)->toBeNull(); }); + +test('a row without a subscription ID is ended when the customer has only canceled subscriptions', function () { + $this->subscription->update(['stripe_subscription_id' => null]); + + $mockStripe = Mockery::mock(StripeClient::class); + $mockSubscriptions = Mockery::mock(SubscriptionService::class); + $mockStripe->subscriptions = $mockSubscriptions; + + // Stripe lists canceled subscriptions only with status "all". + $mockSubscriptions->shouldReceive('all') + ->with(Mockery::on(fn (array $parameters) => ($parameters['status'] ?? null) === 'all')) + ->andReturn((object) ['data' => [(object) ['id' => 'sub_gone', 'status' => 'canceled']]]); + $mockSubscriptions->shouldReceive('retrieve')->with('sub_gone')->andReturn((object) [ + 'status' => 'canceled', + 'cancel_at_period_end' => false, + ]); + + app()->bind(StripeClient::class, fn () => $mockStripe); + + (new VerifyStripeSubscriptionStatusJob($this->subscription))->handle(); + + $this->subscription->refresh(); + expect($this->subscription->stripe_invoice_paid)->toBeFalsy(); +});