Zweite Fix-Runde: Regressionen der ersten (Trockenlauf, Doppelmail, Bestand, Isolierung)
parent
b15fcc53a5
commit
7c4d4fd11c
|
|
@ -43,6 +43,7 @@ class OpenDunningCase
|
|||
// Die erste Mahnung, nicht der Hinweis: der geht sofort hinaus.
|
||||
'next_step_at' => $now->copy()->addDays(DunningSchedule::dayOfLevel(1)),
|
||||
'fee_invoice_ids' => [],
|
||||
'notified_levels' => [],
|
||||
],
|
||||
);
|
||||
|
||||
|
|
@ -52,6 +53,12 @@ class OpenDunningCase
|
|||
// unterscheidet das Eröffnen vom Wiedersehen derselben Rechnung.
|
||||
if ($case->wasRecentlyCreated) {
|
||||
app(DunningMailer::class)->level($case->load('subscription.customer'), 0);
|
||||
|
||||
// VERMERKEN, sonst hält der Nachhol-Lauf des nächsten Tages diese
|
||||
// Nachricht für verloren und schickt dem Kunden dieselbe Mail ein
|
||||
// zweites Mal. Erst nach dem Einreihen: andersherum stünde hier
|
||||
// eine Nachricht, die nie einging.
|
||||
$case->update(['notified_levels' => [0]]);
|
||||
}
|
||||
|
||||
return $case;
|
||||
|
|
|
|||
|
|
@ -47,13 +47,18 @@ class AdvanceDunning extends Command
|
|||
public function handle(StripeClient $stripe): int
|
||||
{
|
||||
$now = Carbon::now();
|
||||
$dryRun = (bool) $this->option('dry-run');
|
||||
|
||||
// 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
|
||||
// Zuerst nachholen, was nie hinausging. 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();
|
||||
//
|
||||
// Im Trockenlauf NICHT. Meine erste Fassung stellte diesen Aufruf vor
|
||||
// die Abfrage auf --dry-run: ein Befehl, der „nur zeigt, was geschähe",
|
||||
// verschickte damit Kundenmails und schrieb in die Datenbank.
|
||||
$this->catchUpNotices($dryRun);
|
||||
|
||||
$faellig = DunningCase::query()
|
||||
->whereNull('settled_at')
|
||||
|
|
@ -69,7 +74,6 @@ class AdvanceDunning extends Command
|
|||
return self::SUCCESS;
|
||||
}
|
||||
|
||||
$dryRun = (bool) $this->option('dry-run');
|
||||
$gestolpert = 0;
|
||||
|
||||
foreach ($faellig as $case) {
|
||||
|
|
@ -112,7 +116,7 @@ class AdvanceDunning extends Command
|
|||
* wurde. Ohne diese Liste liess sich eine verlorene Mail nicht einmal
|
||||
* nachträglich feststellen: sie hinterlässt nirgends eine Spur.
|
||||
*/
|
||||
private function catchUpNotices(): void
|
||||
private function catchUpNotices(bool $dryRun): void
|
||||
{
|
||||
$offen = DunningCase::query()
|
||||
->whereNull('settled_at')
|
||||
|
|
@ -121,10 +125,29 @@ class AdvanceDunning extends Command
|
|||
->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));
|
||||
$kunde = $case->subscription?->customer?->name ?? $case->subscription_id;
|
||||
|
||||
$this->notify($case, $case->level);
|
||||
$this->line(sprintf(' %-28s Nachricht zu Stufe %d %s',
|
||||
$kunde, $case->level, $dryRun ? 'wäre nachgeholt worden' : 'nachgeholt'));
|
||||
|
||||
if ($dryRun) {
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
$this->notify($case, $case->level);
|
||||
} catch (Throwable $e) {
|
||||
// Dieselbe Zusicherung wie beim Weiterrücken: ein Fall, der
|
||||
// stolpert, darf die übrigen nicht liegen lassen. Meine erste
|
||||
// Fassung liess die Ausnahme durch und brach damit den ganzen
|
||||
// Lauf ab — einschliesslich der fälligen Stufen danach.
|
||||
$this->error(sprintf(' %-28s %s', $kunde, $e->getMessage()));
|
||||
Log::error('Eine nachzuholende Mahnungs-Nachricht scheiterte', [
|
||||
'case' => $case->id,
|
||||
'level' => $case->level,
|
||||
'exception' => $e->getMessage(),
|
||||
]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
use Illuminate\Database\Migrations\Migration;
|
||||
use Illuminate\Database\Schema\Blueprint;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Schema;
|
||||
|
||||
/**
|
||||
|
|
@ -20,6 +21,20 @@ return new class extends Migration
|
|||
Schema::table('dunning_cases', function (Blueprint $table) {
|
||||
$table->json('notified_levels')->nullable()->after('fee_invoice_ids');
|
||||
});
|
||||
|
||||
// Bestehende Fälle nachtragen.
|
||||
//
|
||||
// Ohne das hielte der erste Lauf nach dem Update JEDE Nachricht für
|
||||
// verloren und schickte sie erneut — auch Sperrmitteilungen, an Kunden,
|
||||
// die sie längst bekommen haben. Ein Fall, der Stufe 3 erreicht hat,
|
||||
// hat 0 bis 3 bekommen; das ist die einzige Annahme, die hier möglich
|
||||
// ist, und sie ist die richtige: der Lauf, der die Stufe gesetzt hat,
|
||||
// hat auch die Mail eingereiht.
|
||||
foreach (DB::table('dunning_cases')->select('id', 'level')->get() as $case) {
|
||||
DB::table('dunning_cases')
|
||||
->where('id', $case->id)
|
||||
->update(['notified_levels' => json_encode(range(0, (int) $case->level))]);
|
||||
}
|
||||
}
|
||||
|
||||
public function down(): void
|
||||
|
|
|
|||
|
|
@ -216,3 +216,77 @@ it('does not write the same level twice once it went out', function () {
|
|||
|
||||
Mail::assertNothingQueued();
|
||||
});
|
||||
|
||||
// ---- Zweite Fix-Runde: Regressionen der ersten --------------------------
|
||||
|
||||
it('sends nothing at all during a dry run', function () {
|
||||
// Ein Befehl, der „nur zeigt, was geschähe", darf keine Kundenmail
|
||||
// verschicken und nichts speichern. Der Nachhol-Lauf stand vor der
|
||||
// Abfrage auf --dry-run.
|
||||
['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()->subMinute(),
|
||||
'fee_invoice_ids' => [], 'notified_levels' => [],
|
||||
]);
|
||||
|
||||
app()->instance(StripeClient::class, new FakeStripeClient);
|
||||
app()->instance(ProxmoxClient::class, new FakeProxmoxClient);
|
||||
Mail::fake();
|
||||
|
||||
test()->artisan('clupilot:advance-dunning --dry-run')->assertSuccessful();
|
||||
|
||||
Mail::assertNothingQueued();
|
||||
expect($case->fresh()->level)->toBe(1)
|
||||
->and($case->fresh()->notified_levels)->toBe([]);
|
||||
});
|
||||
|
||||
it('records the opening notice so it is not sent a second time', function () {
|
||||
// OpenDunningCase verschickt Stufe 0 sofort, vermerkte sie aber nicht —
|
||||
// der nächste Tageslauf hielt sie für verloren und schickte dieselbe Mail
|
||||
// noch einmal.
|
||||
Mail::fake();
|
||||
customerWithTwoClouds();
|
||||
|
||||
app(ApplyStripeBillingEvent::class)->invoicePaymentFailed([
|
||||
'id' => 'in_neu', 'subscription' => 'sub_schuldig',
|
||||
]);
|
||||
|
||||
$case = DunningCase::query()->where('stripe_invoice_id', 'in_neu')->sole();
|
||||
expect($case->notified_levels)->toBe([0]);
|
||||
|
||||
app()->instance(StripeClient::class, new FakeStripeClient);
|
||||
app()->instance(ProxmoxClient::class, new FakeProxmoxClient);
|
||||
|
||||
test()->artisan('clupilot:advance-dunning');
|
||||
|
||||
Mail::assertQueued(DunningNoticeMail::class, 1);
|
||||
});
|
||||
|
||||
it('does not let one broken case stop the catch-up for the others', function () {
|
||||
// Dieselbe Zusicherung, die das Weiterrücken längst hat: ein Fall, der
|
||||
// stolpert, darf die übrigen nicht liegen lassen.
|
||||
['schuldig' => $schuldig, 'bezahlt' => $zweiter] = customerWithTwoClouds();
|
||||
|
||||
foreach ([[$schuldig, 'in_a'], [$zweiter, 'in_b']] as [$sub, $invoice]) {
|
||||
DunningCase::query()->create([
|
||||
'subscription_id' => $sub->id, 'stripe_invoice_id' => $invoice, '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);
|
||||
|
||||
// Der erste Fall wirft beim Vermerken, der zweite muss trotzdem laufen.
|
||||
DunningCase::query()->orderBy('id')->first()->update(['stripe_invoice_id' => 'in_a']);
|
||||
|
||||
test()->artisan('clupilot:advance-dunning')->assertSuccessful();
|
||||
|
||||
// Beide sind vermerkt — keiner blieb liegen.
|
||||
expect(DunningCase::query()->get()->every(fn ($c) => $c->notified_levels === [1]))->toBeTrue();
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue