diff --git a/app/Actions/Stripe/RefundSubscription.php b/app/Actions/Stripe/RefundSubscription.php index 021cba13e..fc8191f5b 100644 --- a/app/Actions/Stripe/RefundSubscription.php +++ b/app/Actions/Stripe/RefundSubscription.php @@ -99,16 +99,27 @@ public function execute(Team $team): array 'payment_intent' => $paymentIntentId, ]); - $this->stripe->subscriptions->cancel($subscription->stripe_subscription_id); + // Record refund immediately so it cannot be retried if cancel fails + $subscription->update([ + 'stripe_refunded_at' => now(), + 'stripe_feedback' => 'Refund requested by user', + 'stripe_comment' => 'Full refund processed within 30-day window at '.now()->toDateTimeString(), + ]); + + try { + $this->stripe->subscriptions->cancel($subscription->stripe_subscription_id); + } catch (\Exception $e) { + \Log::critical("Refund succeeded but subscription cancel failed for team {$team->id}: ".$e->getMessage()); + send_internal_notification( + "CRITICAL: Refund succeeded but cancel failed for subscription {$subscription->stripe_subscription_id}, team {$team->id}. Manual intervention required." + ); + } $subscription->update([ 'stripe_cancel_at_period_end' => false, 'stripe_invoice_paid' => false, 'stripe_trial_already_ended' => false, 'stripe_past_due' => false, - 'stripe_feedback' => 'Refund requested by user', - 'stripe_comment' => 'Full refund processed within 30-day window at '.now()->toDateTimeString(), - 'stripe_refunded_at' => now(), ]); $team->subscriptionEnded(); diff --git a/app/Actions/Stripe/UpdateSubscriptionQuantity.php b/app/Actions/Stripe/UpdateSubscriptionQuantity.php index c181e988d..a3eab4dca 100644 --- a/app/Actions/Stripe/UpdateSubscriptionQuantity.php +++ b/app/Actions/Stripe/UpdateSubscriptionQuantity.php @@ -153,12 +153,19 @@ public function execute(Team $team, int $quantity): array \Log::warning("Subscription {$subscription->stripe_subscription_id} quantity updated but invoice not paid (status: {$latestInvoice->status}) for team {$team->name}. Reverting to {$previousQuantity}."); // Revert subscription quantity on Stripe - $this->stripe->subscriptions->update($subscription->stripe_subscription_id, [ - 'items' => [ - ['id' => $item->id, 'quantity' => $previousQuantity], - ], - 'proration_behavior' => 'none', - ]); + try { + $this->stripe->subscriptions->update($subscription->stripe_subscription_id, [ + 'items' => [ + ['id' => $item->id, 'quantity' => $previousQuantity], + ], + 'proration_behavior' => 'none', + ]); + } catch (\Exception $revertException) { + \Log::critical("Failed to revert Stripe quantity for subscription {$subscription->stripe_subscription_id}, team {$team->id}. Stripe may have quantity {$quantity} but local is {$previousQuantity}. Error: ".$revertException->getMessage()); + send_internal_notification( + "CRITICAL: Stripe quantity revert failed for subscription {$subscription->stripe_subscription_id}, team {$team->id}. Manual reconciliation required." + ); + } // Void the unpaid invoice if ($latestInvoice->id) { diff --git a/app/Jobs/StripeProcessJob.php b/app/Jobs/StripeProcessJob.php index e61ac81e4..f5d52f29c 100644 --- a/app/Jobs/StripeProcessJob.php +++ b/app/Jobs/StripeProcessJob.php @@ -2,6 +2,7 @@ namespace App\Jobs; +use App\Actions\Stripe\UpdateSubscriptionQuantity; use App\Models\Subscription; use App\Models\Team; use Illuminate\Contracts\Queue\ShouldBeEncrypted; @@ -238,6 +239,7 @@ public function handle(): void 'stripe_invoice_paid' => false, ]); } + break; case 'customer.subscription.updated': $teamId = data_get($data, 'metadata.team_id'); $userId = data_get($data, 'metadata.user_id'); @@ -272,14 +274,14 @@ public function handle(): void $comment = data_get($data, 'cancellation_details.comment'); $lookup_key = data_get($data, 'items.data.0.price.lookup_key'); if (str($lookup_key)->contains('dynamic')) { - $quantity = data_get($data, 'items.data.0.quantity', 2); + $quantity = min((int) data_get($data, 'items.data.0.quantity', 2), UpdateSubscriptionQuantity::MAX_SERVER_LIMIT); $team = data_get($subscription, 'team'); if ($team) { $team->update([ 'custom_server_limit' => $quantity, ]); + ServerLimitCheckJob::dispatch($team); } - ServerLimitCheckJob::dispatch($team); } $subscription->update([ 'stripe_feedback' => $feedback, diff --git a/app/Jobs/VerifyStripeSubscriptionStatusJob.php b/app/Jobs/VerifyStripeSubscriptionStatusJob.php index cf7c3c0ea..f7addacf1 100644 --- a/app/Jobs/VerifyStripeSubscriptionStatusJob.php +++ b/app/Jobs/VerifyStripeSubscriptionStatusJob.php @@ -82,12 +82,9 @@ public function handle(): void 'stripe_past_due' => false, ]); - // Trigger subscription ended logic if canceled - if ($stripeSubscription->status === 'canceled') { - $team = $this->subscription->team; - if ($team) { - $team->subscriptionEnded(); - } + $team = $this->subscription->team; + if ($team) { + $team->subscriptionEnded(); } break; diff --git a/app/Models/Team.php b/app/Models/Team.php index e32526169..10b22b4e1 100644 --- a/app/Models/Team.php +++ b/app/Models/Team.php @@ -197,6 +197,10 @@ public function isAnyNotificationEnabled() public function subscriptionEnded() { + if (! $this->subscription) { + return; + } + $this->subscription->update([ 'stripe_subscription_id' => null, 'stripe_cancel_at_period_end' => false, diff --git a/tests/Feature/Subscription/RefundSubscriptionTest.php b/tests/Feature/Subscription/RefundSubscriptionTest.php index b6c2d4064..144cdad09 100644 --- a/tests/Feature/Subscription/RefundSubscriptionTest.php +++ b/tests/Feature/Subscription/RefundSubscriptionTest.php @@ -251,6 +251,56 @@ expect($result['error'])->toContain('No payment intent'); }); + test('records refund and proceeds when cancel fails', function () { + $stripeSubscription = (object) [ + 'status' => 'active', + 'start_date' => now()->subDays(10)->timestamp, + ]; + + $this->mockSubscriptions + ->shouldReceive('retrieve') + ->with('sub_test_123') + ->andReturn($stripeSubscription); + + $invoiceCollection = (object) ['data' => [ + (object) ['payment_intent' => 'pi_test_123'], + ]]; + + $this->mockInvoices + ->shouldReceive('all') + ->with([ + 'subscription' => 'sub_test_123', + 'status' => 'paid', + 'limit' => 1, + ]) + ->andReturn($invoiceCollection); + + $this->mockRefunds + ->shouldReceive('create') + ->with(['payment_intent' => 'pi_test_123']) + ->andReturn((object) ['id' => 're_test_123']); + + // Cancel throws — simulating Stripe failure after refund + $this->mockSubscriptions + ->shouldReceive('cancel') + ->with('sub_test_123') + ->andThrow(new \Exception('Stripe cancel API error')); + + $action = new RefundSubscription($this->mockStripe); + $result = $action->execute($this->team); + + // Should still succeed — refund went through + expect($result['success'])->toBeTrue(); + expect($result['error'])->toBeNull(); + + $this->subscription->refresh(); + // Refund timestamp must be recorded + expect($this->subscription->stripe_refunded_at)->not->toBeNull(); + // Subscription should still be marked as ended locally + expect($this->subscription->stripe_invoice_paid)->toBeFalsy(); + expect($this->subscription->stripe_subscription_id)->toBeNull(); + }); + test('fails when subscription is past refund window', function () { $stripeSubscription = (object) [ 'status' => 'active', diff --git a/tests/Feature/Subscription/StripeProcessJobTest.php b/tests/Feature/Subscription/StripeProcessJobTest.php new file mode 100644 index 000000000..95cff188a --- /dev/null +++ b/tests/Feature/Subscription/StripeProcessJobTest.php @@ -0,0 +1,143 @@ +set('constants.coolify.self_hosted', false); + config()->set('subscription.provider', 'stripe'); + config()->set('subscription.stripe_api_key', 'sk_test_fake'); + config()->set('subscription.stripe_excluded_plans', ''); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); +}); + +describe('customer.subscription.created does not fall through to updated', function () { + test('created event creates subscription without setting stripe_invoice_paid to true', function () { + Queue::fake(); + + $event = [ + 'type' => 'customer.subscription.created', + 'data' => [ + 'object' => [ + 'customer' => 'cus_new_123', + 'id' => 'sub_new_123', + 'metadata' => [ + 'team_id' => $this->team->id, + 'user_id' => $this->user->id, + ], + ], + ], + ]; + + $job = new StripeProcessJob($event); + $job->handle(); + + $subscription = Subscription::where('team_id', $this->team->id)->first(); + + expect($subscription)->not->toBeNull(); + expect($subscription->stripe_subscription_id)->toBe('sub_new_123'); + expect($subscription->stripe_customer_id)->toBe('cus_new_123'); + // Critical: stripe_invoice_paid must remain false — payment not yet confirmed + expect($subscription->stripe_invoice_paid)->toBeFalsy(); + }); +}); + +describe('customer.subscription.updated clamps quantity to MAX_SERVER_LIMIT', function () { + test('quantity exceeding MAX is clamped to 100', function () { + Queue::fake(); + + Subscription::create([ + 'team_id' => $this->team->id, + 'stripe_subscription_id' => 'sub_existing', + 'stripe_customer_id' => 'cus_clamp_test', + 'stripe_invoice_paid' => true, + ]); + + $event = [ + 'type' => 'customer.subscription.updated', + 'data' => [ + 'object' => [ + 'customer' => 'cus_clamp_test', + 'id' => 'sub_existing', + 'status' => 'active', + 'metadata' => [ + 'team_id' => $this->team->id, + 'user_id' => $this->user->id, + ], + 'items' => [ + 'data' => [[ + 'subscription' => 'sub_existing', + 'plan' => ['id' => 'price_dynamic_monthly'], + 'price' => ['lookup_key' => 'dynamic_monthly'], + 'quantity' => 999, + ]], + ], + 'cancel_at_period_end' => false, + 'cancellation_details' => ['feedback' => null, 'comment' => null], + ], + ], + ]; + + $job = new StripeProcessJob($event); + $job->handle(); + + $this->team->refresh(); + expect($this->team->custom_server_limit)->toBe(100); + + Queue::assertPushed(ServerLimitCheckJob::class); + }); +}); + +describe('ServerLimitCheckJob dispatch is guarded by team check', function () { + test('does not dispatch ServerLimitCheckJob when team is null', function () { + Queue::fake(); + + // Create subscription without a valid team relationship + $subscription = Subscription::create([ + 'team_id' => 99999, + 'stripe_subscription_id' => 'sub_orphan', + 'stripe_customer_id' => 'cus_orphan_test', + 'stripe_invoice_paid' => true, + ]); + + $event = [ + 'type' => 'customer.subscription.updated', + 'data' => [ + 'object' => [ + 'customer' => 'cus_orphan_test', + 'id' => 'sub_orphan', + 'status' => 'active', + 'metadata' => [ + 'team_id' => null, + 'user_id' => null, + ], + 'items' => [ + 'data' => [[ + 'subscription' => 'sub_orphan', + 'plan' => ['id' => 'price_dynamic_monthly'], + 'price' => ['lookup_key' => 'dynamic_monthly'], + 'quantity' => 5, + ]], + ], + 'cancel_at_period_end' => false, + 'cancellation_details' => ['feedback' => null, 'comment' => null], + ], + ], + ]; + + $job = new StripeProcessJob($event); + $job->handle(); + + Queue::assertNotPushed(ServerLimitCheckJob::class); + }); +}); diff --git a/tests/Feature/Subscription/TeamSubscriptionEndedTest.php b/tests/Feature/Subscription/TeamSubscriptionEndedTest.php new file mode 100644 index 000000000..55d59e0e6 --- /dev/null +++ b/tests/Feature/Subscription/TeamSubscriptionEndedTest.php @@ -0,0 +1,16 @@ +create(); + + // Should return early without error — no NPE + $team->subscriptionEnded(); + + // If we reach here, no exception was thrown + expect(true)->toBeTrue(); +}); diff --git a/tests/Feature/Subscription/VerifyStripeSubscriptionStatusJobTest.php b/tests/Feature/Subscription/VerifyStripeSubscriptionStatusJobTest.php new file mode 100644 index 000000000..be8661b6c --- /dev/null +++ b/tests/Feature/Subscription/VerifyStripeSubscriptionStatusJobTest.php @@ -0,0 +1,102 @@ +set('constants.coolify.self_hosted', false); + config()->set('subscription.provider', 'stripe'); + config()->set('subscription.stripe_api_key', 'sk_test_fake'); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + + $this->subscription = Subscription::create([ + 'team_id' => $this->team->id, + 'stripe_subscription_id' => 'sub_verify_123', + 'stripe_customer_id' => 'cus_verify_123', + 'stripe_invoice_paid' => true, + 'stripe_past_due' => false, + ]); +}); + +test('subscriptionEnded is called for unpaid status', function () { + $mockStripe = Mockery::mock(StripeClient::class); + $mockSubscriptions = Mockery::mock(SubscriptionService::class); + $mockStripe->subscriptions = $mockSubscriptions; + + $mockSubscriptions + ->shouldReceive('retrieve') + ->with('sub_verify_123') + ->andReturn((object) [ + 'status' => 'unpaid', + 'cancel_at_period_end' => false, + ]); + + app()->bind(StripeClient::class, fn () => $mockStripe); + + // Create a server to verify it gets disabled + $server = Server::factory()->create(['team_id' => $this->team->id]); + + $job = new VerifyStripeSubscriptionStatusJob($this->subscription); + $job->handle(); + + $this->subscription->refresh(); + expect($this->subscription->stripe_invoice_paid)->toBeFalsy(); + expect($this->subscription->stripe_subscription_id)->toBeNull(); +}); + +test('subscriptionEnded is called for incomplete_expired status', function () { + $mockStripe = Mockery::mock(StripeClient::class); + $mockSubscriptions = Mockery::mock(SubscriptionService::class); + $mockStripe->subscriptions = $mockSubscriptions; + + $mockSubscriptions + ->shouldReceive('retrieve') + ->with('sub_verify_123') + ->andReturn((object) [ + 'status' => 'incomplete_expired', + 'cancel_at_period_end' => false, + ]); + + app()->bind(StripeClient::class, fn () => $mockStripe); + + $job = new VerifyStripeSubscriptionStatusJob($this->subscription); + $job->handle(); + + $this->subscription->refresh(); + expect($this->subscription->stripe_invoice_paid)->toBeFalsy(); + expect($this->subscription->stripe_subscription_id)->toBeNull(); +}); + +test('subscriptionEnded is called for canceled status', function () { + $mockStripe = Mockery::mock(StripeClient::class); + $mockSubscriptions = Mockery::mock(SubscriptionService::class); + $mockStripe->subscriptions = $mockSubscriptions; + + $mockSubscriptions + ->shouldReceive('retrieve') + ->with('sub_verify_123') + ->andReturn((object) [ + 'status' => 'canceled', + 'cancel_at_period_end' => false, + ]); + + app()->bind(StripeClient::class, fn () => $mockStripe); + + $job = new VerifyStripeSubscriptionStatusJob($this->subscription); + $job->handle(); + + $this->subscription->refresh(); + expect($this->subscription->stripe_invoice_paid)->toBeFalsy(); + expect($this->subscription->stripe_subscription_id)->toBeNull(); +});