Write down what the reviews found and nobody fixed

Ten follow-ups, none of them blocking and none of them done. The one that
matters: activePricesFor() recognises an orphaned Price and nothing recognises an
orphaned PRODUCT — and a second Product is worse than a duplicate Price, because
every activePricesFor() then asks about the wrong Product and recognition goes
blind for that whole family. The branch disables its own guard through the gap it
did not close.

Second: AdoptStripePrice promises in its own docblock that no adoption can move
money, but can only compare amount, currency and interval. The four properties
that also decide what a Price charges are filtered in HttpStripeClient, which the
fake cannot express — so no test of the class can reach the promise it makes.

Also recorded: two pre-existing test failures this branch surfaced without
causing, with the diagnosis, so the next person does not derive them again.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feature/betriebsmodus^2
nexxo 2026-07-30 14:02:51 +02:00
parent 3cf16ecd63
commit 4eb90c834a
1 changed files with 85 additions and 0 deletions

View File

@ -272,3 +272,88 @@ Zwei Regeldatei-Tests nach Repo-Sitte, getrennt danach, **gegen wen** sie prüfe
- `app/Services/Billing/PlanPrices.php` — dasselbe
- `docs/handoffs/2026-07-30-real-run-handoff.md` — Block-D-Punkt als erledigt
markieren
---
## 14. Folgepunkte, die aus der Umsetzung entstanden sind
Nachgetragen nach dem Abschluss-Review. Alle Befunde sind offengelegt, keiner
blockiert den Merge — aber keiner ist erledigt.
### Die eine echte Lücke: Waisen-**Produkte**
`activePricesFor()` erkennt einen verwaisten *Preis* wieder. Für ein verwaistes
**Produkt** gibt es kein Gegenstück, und dort ist der Schaden größer: ein
zweites Produkt lässt jedes `activePricesFor($neueId)` leer zurückkommen, damit
ist die Wiedererkennung für dieses Modul oder diese Familie **dauerhaft blind**
und jede Waise am ersten Produkt unerreichbar. Der Zweig deaktiviert seine
eigene Sicherung durch die Lücke, die er nicht geschlossen hat.
Betroffen: `SyncStripeCatalogue::handle()` (Produkt anlegen, ID danach in einem
zweiten Schreibvorgang speichern) und `AddonPrices::product()`. Beide Kommentare
benennen die Lücke jetzt; der Fix wäre ein `productsFor()` nach dem Muster von
`activePricesFor()`, das Stripe nach den Produkten fragt und über die Metadaten
wiedererkennt.
### Das Geld-Tor sitzt an der falschen Stelle
`AdoptStripePrice` verspricht in seinem Kopfkommentar, keine Übernahme könne
Geld verschieben — kann aber nur Betrag, Währung und Intervall vergleichen. Was
sonst noch entscheidet, was ein Preis verrechnet (`interval_count`,
`usage_type`, `transform_quantity`, `billing_scheme`), filtert
`HttpStripeClient::activePricesFor()`. Zwei Folgen: der `FakeStripeClient` kann
diese Felder gar nicht ausdrücken, also kann **kein Test von
`AdoptStripePrice` das Geld-Tor abdecken**; und eine dritte Implementierung von
`StripeClient` ließe es still fallen. Die Prüfung gehört in die Klasse, die sie
zusagt.
Verwandt: `PlanPrices::ensure()` hat keine `<= 0`-Schranke, wo
`AddonPrices::ensure()` eine hat. Das ist das Einzige, was einen
`billing_scheme: tiered`-Preis (bei dem Stripe `unit_amount: null` liefert, hier
als 0 gelesen) auf der Paketseite überhaupt erreichbar macht.
### Kleineres, benannt statt vergessen
- **Der Abgleich zählt „angelegt", auch wo er übernommen hat.** Die Wortwahl der
Konsolenausgabe ist entschärft („created or adopted"), gezählt wird weiterhin
vor dem Aufruf. Eine echte Trennung der beiden Zahlen fehlt — und das ist die
eine Zahl, die ein Mensch nach einem Vorfall zuerst liest.
- **Die Warnung über einen unerklärlichen Preis hat keine Drosselung.** Sie
feuert bei jedem Abgleich *und* jeder Kundenbuchung, solange die Waise
existiert — und weil das Aufräum-Kommando (§12) ein Folgepunkt ist, auf
unbestimmte Zeit. Der Widerspruchs-Pfad wurde aus genau diesem Grund still
gestellt; der Bestätigungs-Pfad bekam dieselbe Überlegung nicht.
- **`FakeStripeClient::updatePriceMetadata()` ersetzt, wo Stripe zusammenführt**,
und `activePricesFor()` gibt Metadaten ungecastet zurück, wo der HTTP-Client
auf String castet. Heute nicht beobachtbar, aber es ist genau die Sorte
Untreue des Fakes, gegen die §9 geschrieben wurde.
- **`created` im Fake kann gleichstehen**: `createPrice()` stempelt
`count($prices) + 1`, `plantPrice()` hat die Vorgabe `1`. Der Kommentar sagt
das jetzt zu, statt es zu bestreiten; ein Gleichstand-Tiebreak auf die ID
fehlt weiter.
- **`HttpStripeClient::activePricesFor()` bricht das Blättern still ab**, wenn
dem letzten Eintrag einer nicht-letzten Seite die `id` fehlt — getreue Kopie
desselben Verhaltens in `invoiceLines()`. Die Folge hat sich aber geändert: in
`invoiceLines()` entsteht ein zu kurzer Belegtext, hier eine ungesehene Waise
und ein zweiter aktiver Preis, also der Vorfall. Wer es anfasst, soll werfen
statt anhalten.
- **Der Idempotenz-Fingerabdruck und der POST-Rumpf müssen zusammenbleiben.**
Ein Feld, das künftig in `createPrice()` gesendet, aber nicht in
`IdempotencyKey::priceParameters()` aufgenommen wird, öffnet den Vorfall
wieder. Der Test „does not block a booking because the metadata format moved"
wacht über diese Nahtstelle — mit einem gesetzten Ledger-Eintrag, weil der
Zustand heute nicht mehr fahrbar ist.
### Zwei fremde, vorbestehende Testfehler, die dieser Zweig aufgedeckt hat
Keiner davon wird von diesem Zweig verursacht.
1. **`HostStepsTest.php:1064`** schreibt `fsn-01.node.clupilot.com` hart hinein,
während `.env:107` `CLUPILOT_DNS_ZONE=clupilot.cloud` setzt und `phpunit.xml`
neun andere Variablen mit `force="true"` festnagelt, diese aber nicht. Eine
Zeile in `phpunit.xml`. Derselbe Punkt steht als offene Aufgabe in
`docs/handoffs/2026-07-30-real-run-handoff.md`.
2. **`PlanCatalogueTest.php`** ist einmal ausgefallen und lief beim
Wiederholen durch. „Beim zweiten Mal grün" ist keine Diagnose, und ein
sprunghafter Fehler in den Katalogpreis-Tests liegt im Wirkungskreis dieses
Zweigs, auch wenn er nicht von ihm kommt.