Fix-Runde: Reichweite der Sperre, keine doppelte Gebuehr, verlorene Nachricht

main
nexxo 2026-07-31 22:57:56 +02:00
parent 4f037c8264
commit b15fcc53a5
10 changed files with 398 additions and 21 deletions

View File

@ -2,8 +2,8 @@
namespace App\Actions;
use App\Console\Commands\AdvanceDunning;
use App\Models\DunningCase;
use App\Models\Instance;
use App\Services\Billing\DunningMailer;
use App\Services\Billing\DunningSchedule;
use App\Services\Stripe\StripeClient;
@ -37,7 +37,15 @@ class SettleDunningCase
}
$offen = array_column(app(StripeClient::class)->openInvoices((string) $customerId), 'id');
$unsere = array_merge([$case->stripe_invoice_id], array_values($case->fee_invoice_ids ?? []));
// Platzhalter zählen nicht mit: sie sind keine Stripe-ID, und wogegen
// man sie prüfen sollte, weiss niemand. Sie stehen im Fall und auf der
// Seite — dort kann ein Mensch nachsehen. Sie hier als „offen" zu
// werten hiesse, den Fall für immer festzuhalten.
$unsere = array_values(array_filter(
array_merge([$case->stripe_invoice_id], array_values($case->fee_invoice_ids ?? [])),
fn (string $id) => $id !== AdvanceDunning::PENDING,
));
// Steht noch eine davon offen, ist der Fall nicht zu.
if (array_intersect($unsere, $offen) !== []) {
@ -48,12 +56,15 @@ class SettleDunningCase
$stand = $case->level;
// Und die Cloud kommt zurück. Nur die des betroffenen Vertrags —
// andere Verträge desselben Kunden waren nie betroffen.
foreach (Instance::query()
->where('customer_id', $case->subscription?->customer_id)
->whereNotNull('suspended_at')
->get() as $instance) {
// Und die Cloud kommt zurück — NUR die dieses Vertrags.
//
// Vorher holte eine Zahlung auf Vertrag A jede gesperrte Cloud des
// Kunden zurück, auch die, für die noch geschuldet wird (Codex P1).
// Ein Kunde mit zwei Verträgen hätte durch das Begleichen des einen
// den anderen freibekommen.
$instance = $case->instance();
if ($instance !== null && $instance->suspended_at !== null) {
app(ResumeInstance::class)($instance);
}

View File

@ -4,7 +4,6 @@ namespace App\Console\Commands;
use App\Actions\SuspendInstance;
use App\Models\DunningCase;
use App\Models\Instance;
use App\Services\Billing\DunningMailer;
use App\Services\Billing\DunningSchedule;
use App\Services\Stripe\StripeClient;
@ -36,10 +35,26 @@ class AdvanceDunning extends Command
protected $description = 'Fällige Mahnfälle eine Stufe weiterschalten';
/**
* Was in `fee_invoice_ids` steht, solange die Antwort von Stripe fehlt.
*
* Der Eintrag entsteht VOR dem Aufruf und verhindert damit eine zweite
* Buchung derselben Stufe auch dann, wenn niemand mehr erfährt, welche
* Rechnung dabei entstanden ist.
*/
public const PENDING = 'pending';
public function handle(StripeClient $stripe): int
{
$now = Carbon::now();
// Zuerst nachholen, was nie hinausging (Codex P2). Ein Fall, dessen
// Mail scheiterte, rückte trotzdem weiter — der Kunde wurde gemahnt
// und später abgeschaltet, ohne es je zu erfahren. Das läuft VOR dem
// Weiterrücken, damit eine nachgeholte Nachricht nicht sofort von der
// nächsten Stufe überholt wird.
$this->catchUpNotices();
$faellig = DunningCase::query()
->whereNull('settled_at')
->where('level', '<', DunningSchedule::SUSPENDED)
@ -90,6 +105,48 @@ class AdvanceDunning extends Command
return $gestolpert === 0 ? self::SUCCESS : self::FAILURE;
}
/**
* Nachrichten, die nie hinausgingen, nachreichen.
*
* `notified_levels` hält fest, welche Stufe dem Kunden wirklich mitgeteilt
* wurde. Ohne diese Liste liess sich eine verlorene Mail nicht einmal
* nachträglich feststellen: sie hinterlässt nirgends eine Spur.
*/
private function catchUpNotices(): void
{
$offen = DunningCase::query()
->whereNull('settled_at')
->with('subscription.customer')
->get()
->filter(fn (DunningCase $case) => ! in_array($case->level, $case->notified_levels ?? [], true));
foreach ($offen as $case) {
$this->line(sprintf(' %-28s Nachricht zu Stufe %d nachgeholt',
$case->subscription?->customer?->name ?? $case->subscription_id, $case->level));
$this->notify($case, $case->level);
}
}
/**
* Die Nachricht zu einer Stufe, und der Vermerk darüber.
*
* Vermerkt wird erst NACH dem Einreihen: andersherum stünde in der Liste
* eine Nachricht, die nie einging, und niemand holte sie je nach.
*/
private function notify(DunningCase $case, int $level): void
{
$mailer = app(DunningMailer::class);
$level >= DunningSchedule::SUSPENDED
? $mailer->suspended($case)
: $mailer->level($case, $level);
$case->update([
'notified_levels' => array_values(array_unique(array_merge($case->notified_levels ?? [], [$level]))),
]);
}
private function advance(DunningCase $case, int $level, StripeClient $stripe): void
{
$gebuehr = DunningSchedule::feeCents($level);
@ -102,23 +159,38 @@ class AdvanceDunning extends Command
$kunde = $case->subscription?->customer?->stripe_customer_id;
if (filled($kunde)) {
// Die Stufe wird VERMERKT, BEVOR Stripe gefragt wird
// (Codex P1). Stirbt der Prozess zwischen dem Anlegen und dem
// Speichern, sähe der nächste Lauf sonst eine unvermerkte
// Stufe und buchte dieselbe Mahngebühr ein zweites Mal. Fünf
// Euro doppelt sind wenig Geld und viel Vertrauensverlust —
// eine nicht verrechnete Gebühr ist der günstigere Ausgang.
$rechnungen[(string) $level] = self::PENDING;
$case->update(['fee_invoice_ids' => $rechnungen]);
$rechnungen[(string) $level] = $stripe->createFeeInvoice(
(string) $kunde,
$gebuehr,
'EUR',
__('dunning.fee_description', ['level' => $level]),
// Deckt zusätzlich den Wiederholungsversuch innerhalb von
// 24 Stunden ab: Stripe antwortet dann mit derselben
// Rechnung statt einer zweiten.
idempotencyKey: sprintf('clupilot-dunning-fee-%d-%d', $case->id, $level),
);
}
}
// Die letzte Stufe schaltet ab. Nur die Clouds des betroffenen Kunden,
// und gelöscht wird nichts — siehe SuspendInstance.
// NUR die Cloud dieses Vertrags. Vorher traf es jede Cloud des
// Kunden — ein zweiter, bezahlter Vertrag stand mit, und das ist der
// Anruf, den man nicht will. Ausdrücklich so vereinbart: die Sperre
// trifft den betroffenen Vertrag, nicht das Konto.
if ($level >= DunningSchedule::SUSPENDED) {
foreach (Instance::query()
->where('customer_id', $case->subscription?->customer_id)
->whereNull('suspended_at')
->where('status', 'active')
->get() as $instance) {
$instance = $case->instance();
if ($instance !== null && $instance->suspended_at === null) {
app(SuspendInstance::class)($instance);
}
}
@ -137,7 +209,6 @@ class AdvanceDunning extends Command
// Erst wenn der Zustand steht, dann die Nachricht. Andersherum stünde
// im Postfach des Kunden eine Sperre, die es nicht gibt, weil das
// Speichern danach scheiterte.
$mailer = app(DunningMailer::class);
$gesperrt ? $mailer->suspended($case) : $mailer->level($case, $level);
$this->notify($case->refresh(), $level);
}
}

View File

@ -14,7 +14,7 @@ class DunningCase extends Model
{
protected $fillable = [
'subscription_id', 'stripe_invoice_id', 'level',
'opened_at', 'next_step_at', 'settled_at', 'fee_invoice_ids', 'note',
'opened_at', 'next_step_at', 'settled_at', 'fee_invoice_ids', 'notified_levels', 'note',
];
protected function casts(): array
@ -25,6 +25,7 @@ class DunningCase extends Model
'next_step_at' => 'datetime',
'settled_at' => 'datetime',
'fee_invoice_ids' => 'array',
'notified_levels' => 'array',
];
}
@ -33,6 +34,32 @@ class DunningCase extends Model
return $this->belongsTo(Subscription::class);
}
/**
* Die Cloud, um die es in diesem Fall geht und nur diese.
*
* EINE Stelle für die Frage, weil Sperre und Freigabe sie beide stellen und
* zwei Antworten darauf der Weg wären, auf dem sie auseinanderlaufen: eine
* Cloud, die abgeschaltet wird und nie zurückkommt.
*
* Über die BESTELLUNG, nicht über `subscriptions.instance_id`. Die Spalte
* gibt es, aber OpenSubscription füllt sie nicht auf dieser Installation
* war sie am 31.7.2026 bei drei von fünf Verträgen leer. Eine Sperre, die
* daran hängt, wäre für die meisten Kunden ein stiller Nulleffekt gewesen:
* nie abgeschaltet, nie eine Meldung. `order_id` war bei keinem Vertrag und
* keiner Instanz leer.
*/
public function instance(): ?Instance
{
$subscription = $this->subscription;
if ($subscription === null) {
return null;
}
return $subscription->instance
?? Instance::query()->where('order_id', $subscription->order_id)->first();
}
/** Läuft noch — weder beglichen noch von Hand geschlossen. */
public function isOpen(): bool
{

View File

@ -647,6 +647,7 @@ class FakeStripeClient implements StripeClient
int $amountCents,
string $currency,
string $description,
?string $idempotencyKey = null,
): string {
$this->feeInvoices[] = [
'customer' => $customerId,
@ -656,6 +657,7 @@ class FakeStripeClient implements StripeClient
// Aus der Beschreibung gezogen, damit ein Test die Stufe prüfen
// kann, ohne dass der Fake sie gesondert übergeben bekäme.
'level' => preg_match('/(\d+)/', $description, $m) ? (int) $m[1] : null,
'idempotency_key' => $idempotencyKey,
];
return 'in_fee_'.count($this->feeInvoices);

View File

@ -657,10 +657,13 @@ class HttpStripeClient implements StripeClient
int $amountCents,
string $currency,
string $description,
?string $idempotencyKey = null,
): string {
// Erst der Posten, dann die Rechnung: Stripe sammelt offene Posten
// eines Kunden ein, wenn eine Rechnung entsteht.
$this->request()
// Ein BARER Schlüssel, kein Fingerabdruck der Parameter — dieselbe
// Wahl wie bei refund() und cancelSubscription(): hier ist ein zweites
// Objekt das Geld eines Kunden ein zweites Mal, und Stripes Weigerung
// ist dann die richtige Antwort. Siehe IdempotencyKey.
$this->request($idempotencyKey)
->asForm()
->post($this->url('invoiceitems'), [
'customer' => $customerId,
@ -670,7 +673,7 @@ class HttpStripeClient implements StripeClient
])
->throw();
$invoice = $this->request()
$invoice = $this->request($idempotencyKey !== null ? $idempotencyKey.'-invoice' : null)
->asForm()
->post($this->url('invoices'), [
'customer' => $customerId,

View File

@ -396,5 +396,6 @@ interface StripeClient
int $amountCents,
string $currency,
string $description,
?string $idempotencyKey = null,
): string;
}

View File

@ -0,0 +1,31 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
/**
* Welche Mahnstufen dem Kunden tatsächlich mitgeteilt wurden.
*
* Codex-Review 31.7.2026, P2: der Fall rückte weiter, die Mail scheiterte, und
* kein späterer Lauf holte sie nach der Kunde wurde gemahnt und später
* abgeschaltet, ohne es je erfahren zu haben. Ohne diese Liste liess sich das
* auch nicht nachträglich feststellen: eine nicht verschickte Mail hinterlässt
* nirgends eine Spur.
*/
return new class extends Migration
{
public function up(): void
{
Schema::table('dunning_cases', function (Blueprint $table) {
$table->json('notified_levels')->nullable()->after('fee_invoice_ids');
});
}
public function down(): void
{
Schema::table('dunning_cases', function (Blueprint $table) {
$table->dropColumn('notified_levels');
});
}
};

View File

@ -0,0 +1,218 @@
<?php
use App\Actions\ApplyStripeBillingEvent;
use App\Actions\SuspendInstance;
use App\Mail\DunningNoticeMail;
use App\Models\Customer;
use App\Models\DunningCase;
use App\Models\Instance;
use App\Models\Order;
use App\Models\Subscription;
use App\Services\Billing\DunningSchedule;
use App\Services\Proxmox\FakeProxmoxClient;
use App\Services\Proxmox\ProxmoxClient;
use App\Services\Stripe\FakeStripeClient;
use App\Services\Stripe\StripeClient;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Mail;
/**
* Fix-Runde zum Codex-Review vom 31.7.2026 zwei P1, ein P2, und ein dritter
* Befund, den der Review nicht genannt hat: das ABSCHALTEN war genauso zu weit
* gefasst wie das Wiederhochfahren. Beide trafen jede Cloud des Kunden, obwohl
* ausdrücklich vereinbart war, dass nur der betroffene Vertrag betroffen ist.
*/
function customerWithTwoClouds(): array
{
$customer = Customer::factory()->create(['name' => 'Berger GmbH', 'stripe_customer_id' => 'cus_42']);
$machen = function (string $subId, int $vmid) use ($customer) {
$order = Order::factory()->create(['customer_id' => $customer->id, 'plan' => 'start']);
$instance = Instance::factory()->create([
'customer_id' => $customer->id, 'order_id' => $order->id,
'plan' => 'start', 'status' => 'active', 'vmid' => $vmid,
]);
$subscription = Subscription::factory()->create([
'customer_id' => $customer->id, 'order_id' => $order->id,
'instance_id' => $instance->id,
'stripe_subscription_id' => $subId, 'price_cents' => 4900,
]);
return [$subscription, $instance];
};
[$schuldig, $schuldigeCloud] = $machen('sub_schuldig', 101);
[$bezahlt, $bezahlteCloud] = $machen('sub_bezahlt', 102);
return compact('customer', 'schuldig', 'schuldigeCloud', 'bezahlt', 'bezahlteCloud');
}
// ---- P1 (2) und der ungenannte Befund: die Reichweite -------------------
it('shuts down only the cloud of the contract in arrears', function () {
// Der ungenannte Befund. Ein Kunde mit zwei Verträgen, einer davon
// überfällig — der zweite ist bezahlt und darf nicht mitstehen. Genau der
// Anruf, den man nicht will.
Mail::fake();
['schuldig' => $schuldig, 'schuldigeCloud' => $aus, 'bezahlteCloud' => $laeuft] = customerWithTwoClouds();
DunningCase::query()->create([
'subscription_id' => $schuldig->id, 'stripe_invoice_id' => 'in_1',
'level' => DunningSchedule::SUSPENDED - 1,
'opened_at' => Carbon::now()->subDays(17),
'next_step_at' => Carbon::now()->subMinute(), 'fee_invoice_ids' => [],
]);
app()->instance(ProxmoxClient::class, new FakeProxmoxClient);
app()->instance(StripeClient::class, new FakeStripeClient);
test()->artisan('clupilot:advance-dunning')->assertSuccessful();
expect($aus->fresh()->suspended_at)->not->toBeNull()
->and($laeuft->fresh()->suspended_at)->toBeNull();
});
it('does not bring a cloud back while another case still has it stopped', function () {
// Codex P1: eine Zahlung auf Vertrag A holte jede gesperrte Cloud des
// Kunden zurück — auch die, für die noch geschuldet wird.
Mail::fake();
$w = customerWithTwoClouds();
foreach ([['schuldig', 'in_a'], ['bezahlt', 'in_b']] as [$key, $invoice]) {
DunningCase::query()->create([
'subscription_id' => $w[$key]->id, 'stripe_invoice_id' => $invoice,
'level' => DunningSchedule::SUSPENDED,
'opened_at' => Carbon::now()->subDays(24),
'next_step_at' => null, 'fee_invoice_ids' => [],
]);
}
app()->instance(ProxmoxClient::class, new FakeProxmoxClient);
app(SuspendInstance::class)($w['schuldigeCloud']);
app(SuspendInstance::class)($w['bezahlteCloud']);
// Nur die Rechnung des einen Vertrags ist bezahlt.
$stripe = new FakeStripeClient;
$stripe->openInvoiceRows = [[
'id' => 'in_a', 'number' => 'R-A', 'amount_due_cents' => 4900,
'currency' => 'EUR', 'created_at' => '1750000000',
]];
app()->instance(StripeClient::class, $stripe);
app(ApplyStripeBillingEvent::class)->invoicePaid([
'id' => 'in_b', 'subscription' => 'sub_bezahlt', 'billing_reason' => 'subscription_cycle',
]);
expect($w['bezahlteCloud']->fresh()->suspended_at)->toBeNull()
// Die andere bleibt aus — für sie wird noch geschuldet.
->and($w['schuldigeCloud']->fresh()->suspended_at)->not->toBeNull();
});
// ---- P1 (1): keine zweite Gebühr für dieselbe Stufe ---------------------
it('records the level before it asks Stripe for the fee', function () {
// Codex P1: Stripe legt die Rechnung an, der Prozess stirbt, das Speichern
// kommt nie — und der nächste Lauf bucht dieselbe Mahnstufe ein zweites
// Mal. Fünf Euro doppelt sind wenig Geld und viel Vertrauensverlust.
Mail::fake();
['schuldig' => $schuldig] = customerWithTwoClouds();
$case = DunningCase::query()->create([
'subscription_id' => $schuldig->id, 'stripe_invoice_id' => 'in_1', 'level' => 1,
'opened_at' => Carbon::now()->subDays(10),
'next_step_at' => Carbon::now()->subMinute(), 'fee_invoice_ids' => [],
]);
$stripe = new class extends FakeStripeClient
{
public function createFeeInvoice(string $customerId, int $amountCents, string $currency, string $description, ?string $idempotencyKey = null): string
{
parent::createFeeInvoice($customerId, $amountCents, $currency, $description, $idempotencyKey);
// Genau der Absturz: Stripe hat angelegt, wir kommen nicht mehr
// zum Speichern des Ergebnisses.
throw new RuntimeException('Prozess gestorben nach dem Anlegen');
}
};
app()->instance(StripeClient::class, $stripe);
app()->instance(ProxmoxClient::class, new FakeProxmoxClient);
test()->artisan('clupilot:advance-dunning');
// Die Stufe ist vermerkt, obwohl der Lauf gestorben ist.
expect(array_key_exists('2', $case->fresh()->fee_invoice_ids ?? []))->toBeTrue();
// Und ein zweiter Lauf bucht NICHT noch einmal.
$case->fresh()->update(['next_step_at' => Carbon::now()->subMinute()]);
$zweiter = new FakeStripeClient;
app()->instance(StripeClient::class, $zweiter);
test()->artisan('clupilot:advance-dunning');
expect($zweiter->feeInvoices)->toBe([]);
});
it('sends an idempotency key with the fee so a same-day retry cannot double it', function () {
Mail::fake();
['schuldig' => $schuldig] = customerWithTwoClouds();
$case = DunningCase::query()->create([
'subscription_id' => $schuldig->id, 'stripe_invoice_id' => 'in_1', 'level' => 1,
'opened_at' => Carbon::now()->subDays(10),
'next_step_at' => Carbon::now()->subMinute(), 'fee_invoice_ids' => [],
]);
$stripe = new FakeStripeClient;
app()->instance(StripeClient::class, $stripe);
app()->instance(ProxmoxClient::class, new FakeProxmoxClient);
test()->artisan('clupilot:advance-dunning');
expect($stripe->feeInvoices[0]['idempotency_key'])->toBe('clupilot-dunning-fee-'.$case->id.'-2');
});
// ---- P2: eine verlorene Nachricht wird nachgeholt -----------------------
it('retries a notice that never made it out', function () {
// Codex P2: der Fall rückte weiter, die Mail scheiterte, und kein späterer
// Lauf holte sie nach — der Kunde wurde gemahnt, ohne es zu erfahren.
['schuldig' => $schuldig] = customerWithTwoClouds();
$case = DunningCase::query()->create([
'subscription_id' => $schuldig->id, 'stripe_invoice_id' => 'in_1', 'level' => 1,
'opened_at' => Carbon::now()->subDays(3),
'next_step_at' => Carbon::now()->addDays(7),
'fee_invoice_ids' => [], 'notified_levels' => [],
]);
app()->instance(StripeClient::class, new FakeStripeClient);
app()->instance(ProxmoxClient::class, new FakeProxmoxClient);
Mail::fake();
test()->artisan('clupilot:advance-dunning')->assertSuccessful();
// Nicht fällig, aber die Nachricht zur AKTUELLEN Stufe fehlt — also wird
// sie nachgeholt, ohne dass der Fall weiterrückt.
Mail::assertQueued(DunningNoticeMail::class, fn ($mail) => $mail->level === 1);
expect($case->fresh()->level)->toBe(1)
->and($case->fresh()->notified_levels)->toContain(1);
});
it('does not write the same level twice once it went out', function () {
['schuldig' => $schuldig] = customerWithTwoClouds();
DunningCase::query()->create([
'subscription_id' => $schuldig->id, 'stripe_invoice_id' => 'in_1', 'level' => 1,
'opened_at' => Carbon::now()->subDays(3),
'next_step_at' => Carbon::now()->addDays(7),
'fee_invoice_ids' => [], 'notified_levels' => [1],
]);
app()->instance(StripeClient::class, new FakeStripeClient);
app()->instance(ProxmoxClient::class, new FakeProxmoxClient);
Mail::fake();
test()->artisan('clupilot:advance-dunning');
Mail::assertNothingQueued();
});

View File

@ -34,6 +34,7 @@ function mailableCase(int $level = 0): DunningCase
]);
$subscription = Subscription::factory()->create([
'customer_id' => $customer->id,
'order_id' => $order->id,
'stripe_subscription_id' => 'sub_42',
'stripe_status' => 'past_due',
'price_cents' => 21480,
@ -46,6 +47,11 @@ function mailableCase(int $level = 0): DunningCase
'opened_at' => Carbon::now()->subDays(DunningSchedule::dayOfLevel($level)),
'next_step_at' => Carbon::now()->subMinute(),
'fee_invoice_ids' => [],
// Alles bis zur aktuellen Stufe wurde bereits mitgeteilt — so sieht ein
// Fall im Betrieb aus. Ohne diese Zeile hielte der Nachhol-Lauf die
// Nachricht zur aktuellen Stufe für verloren und schickte sie zu Recht
// nach.
'notified_levels' => range(0, $level),
]);
}

View File

@ -90,8 +90,11 @@ it('starts the machine again and clears the mark', function () {
function suspendedCase(): DunningCase
{
$instance = suspendableInstance();
// Über die BESTELLUNG verknüpft, wie im Betrieb: OpenSubscription füllt
// `instance_id` nicht, `order_id` steht bei jedem Vertrag.
$subscription = Subscription::factory()->create([
'customer_id' => $instance->customer_id,
'order_id' => $instance->order_id,
'stripe_subscription_id' => 'sub_42',
'stripe_status' => 'past_due',
]);
@ -153,6 +156,7 @@ it('settles a case that never got as far as a suspension', function () {
$instance = suspendableInstance();
$subscription = Subscription::factory()->create([
'customer_id' => $instance->customer_id,
'order_id' => $instance->order_id,
'stripe_subscription_id' => 'sub_42',
]);
$case = DunningCase::query()->create([
@ -178,8 +182,11 @@ it('shuts the cloud down when the last level is reached', function () {
// Stufe 4 stand bis hierher nur im Zeitplan. Ohne diesen Anschluss wäre
// die Sperre eine Zahl in einer Spalte, und die Cloud liefe weiter.
$instance = suspendableInstance();
// Über die BESTELLUNG verknüpft, wie im Betrieb: OpenSubscription füllt
// `instance_id` nicht, `order_id` steht bei jedem Vertrag.
$subscription = Subscription::factory()->create([
'customer_id' => $instance->customer_id,
'order_id' => $instance->order_id,
'stripe_subscription_id' => 'sub_42',
'stripe_status' => 'past_due',
]);