Design the nine things the reviews found and nobody built
The one that matters is orphan PRODUCTS. A price orphan is recognised now; a product orphan is not, and it is worse — a second product makes every activePricesFor() ask about the wrong one, so price recognition goes blind for that whole family and the guard disables itself through the gap beside it. Duplicated products are reported, not deactivated. That is the owner's call and it is the careful one: an archived PRICE is provably harmless, which is what the grandfathering rule has always rested on, but deactivating a PRODUCT makes its prices unsellable and contracts can be running on those. What was proven for one is not proven for the other. The money gate moves into the class whose docblock promises it. Today AdoptStripePrice can compare three properties and the other four are filtered in one client, which the fake cannot express — so no test can reach the promise the class makes. Also: a sweep command for orphans nobody can adopt, a day's throttle on the warning that currently fires on every customer booking, and six mechanical fixes. Self-review caught one of my own: the plan to tell "created" from "adopted" by comparing the price id before and after the call cannot work — there is no id before, in either case. AdoptStripePrice counts its own adoptions instead, which needs it bound as a singleton, and the spec now says why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>feature/host-bootstrap
nexxo
parent
48581c3978
commit
3ceec8fe5e
|
|
@ -0,0 +1,243 @@
|
|||
# Spec — Die offenen Punkte der Preis-Wiedererkennung
|
||||
|
||||
**Datum:** 2026-07-30
|
||||
**Status:** entworfen, noch nicht umgesetzt
|
||||
**Vorgänger:** `docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md`, §14
|
||||
|
||||
---
|
||||
|
||||
## 1. Ausgangslage
|
||||
|
||||
Die Preis-Wiedererkennung ist gebaut und gemergt: ein Lauf, der zwischen Stripes
|
||||
Anlage und unserem Schreiben abbricht, hinterlässt keine zweite Preis-Zeile
|
||||
mehr, und eine Änderung an den Metadaten blockiert nichts mehr für 24 Stunden.
|
||||
|
||||
§14 des Vorgängers hat aufgeschrieben, was die Prüfungen dabei fanden und
|
||||
niemand behoben hat. Das wird hier gebaut. Drei Punkte mit Entwurfsentscheidung,
|
||||
sechs mechanische.
|
||||
|
||||
## 2. Waisen-**Produkte** — die eine echte Lücke
|
||||
|
||||
`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 sind zwei Stellen, beide mit demselben Muster — Produkt anlegen, ID in
|
||||
einem **zweiten** Schreibvorgang speichern:
|
||||
|
||||
- `SyncStripeCatalogue::handle()` für Paketfamilien (`$family->update([…])`),
|
||||
- `AddonPrices::product()` für Module (die ID landet erst über `remember()` in
|
||||
einer Preiszeile).
|
||||
|
||||
### Wie es gebaut wird
|
||||
|
||||
`StripeClient::activeProducts(): array` — alle aktiven Produkte des Kontos, nach
|
||||
dem Blätter-Muster von `activePricesFor()`. **Ohne** Filter auf Metadaten: Stripe
|
||||
kann danach nicht suchen, und ein Konto hat eine Handvoll Produkte, keine
|
||||
Tausende.
|
||||
|
||||
`App\Services\Billing\AdoptStripeProduct` — der Wiedererkennungsschritt, nach
|
||||
dem Vorbild von `AdoptStripePrice`, aber deutlich kürzer. Ein Produkt hat **keinen
|
||||
Betrag**, also gibt es hier **kein Geld-Tor**: der Beweis liegt allein in den
|
||||
Metadaten. Übernommen wird ein Produkt, das
|
||||
|
||||
1. bei Stripe **aktiv** ist,
|
||||
2. **mindestens einen tragenden Metadatenschlüssel bestätigt** —
|
||||
`plan_family_id` bei Familien, `addon` bei Modulen — und
|
||||
3. **keinem** unserer Metadatenschlüssel widerspricht, den es trägt.
|
||||
|
||||
Eine Beanspruchungs-Prüfung wie bei den Preisen braucht es nicht: die
|
||||
tragenden Schlüssel sind je Familie und je Modul eindeutig, ein Produkt kann
|
||||
also gar nicht zu zweien gehören.
|
||||
|
||||
### Doppelte Produkte werden gemeldet, nicht stillgelegt
|
||||
|
||||
Entscheidung des Auftraggebers vom 2026-07-30, und sie ist die vorsichtige:
|
||||
**das älteste wird übernommen, die übrigen werden gemeldet und nicht angefasst.**
|
||||
|
||||
Der Grund ist ein echter Unterschied zur Preis-Seite. Ein archivierter *Preis*
|
||||
ist nachweislich harmlos — Stripe verrechnet laufende Abos weiter, nur verkauft
|
||||
wird er nicht mehr, und darauf beruht die Grandfathering-Regel dieses Projekts
|
||||
schon lange. Ein stillgelegtes *Produkt* macht dagegen **seine Preise
|
||||
unverkäuflich**, und auf denen können Verträge laufen. Was bei Preisen bewiesen
|
||||
harmlos war, ist hier nicht bewiesen — also wird es nicht getan.
|
||||
|
||||
Gemeldet wird über `Log::warning` **und** in der Ausgabe von
|
||||
`stripe:sync-catalogue`, weil die Produkt-Anlage im Gegensatz zu `ensure()` nur
|
||||
im Kommando läuft und ein Betreiber dort hinsieht.
|
||||
|
||||
## 3. Das Geld-Tor zieht dorthin, wo es zugesagt wird
|
||||
|
||||
`AdoptStripePrice` verspricht im Kopfkommentar, keine Übernahme könne Geld
|
||||
verschieben — vergleicht aber nur Betrag, Währung und Intervall. Die vier
|
||||
übrigen Eigenschaften, die entscheiden, was ein Preis verrechnet
|
||||
(`interval_count`, `usage_type`, `transform_quantity`, `billing_scheme`),
|
||||
filtert `HttpStripeClient::activePricesFor()`.
|
||||
|
||||
Zwei Folgen, und die zweite ist die schlimmere:
|
||||
|
||||
- der `FakeStripeClient` kann diese Felder gar nicht ausdrücken, also kann
|
||||
**kein Test von `AdoptStripePrice` das Geld-Tor erreichen** — genau die
|
||||
Zusage, um die es geht;
|
||||
- eine dritte Implementierung von `StripeClient` ließe die Prüfung still
|
||||
fallen.
|
||||
|
||||
### Wie es gebaut wird
|
||||
|
||||
`activePricesFor()` **filtert nicht mehr**, sondern gibt die vier Eigenschaften
|
||||
im Rückgabe-Array mit: `interval_count`, `usage_type`, `transform_quantity`
|
||||
(als `bool`: trägt der Preis eine?), `billing_scheme`. Absente Schlüssel werden
|
||||
weiterhin auf Stripes Vorgaben gesetzt — `1`, `licensed`, `false`, `per_unit` —
|
||||
und das bleibt die Richtung, auf die es ankommt: andersherum gelesen würde der
|
||||
Filter jeden legitimen Preis ablehnen und die Wiedererkennung still zu einem
|
||||
Nichts machen.
|
||||
|
||||
`AdoptStripePrice` prüft sie neben Betrag, Währung und Intervall. Der
|
||||
`FakeStripeClient` bekommt die vier Felder in `plantPrice()`, damit ein Test
|
||||
einen vierteljährlichen oder mengenteilenden Preis überhaupt hinstellen kann.
|
||||
|
||||
`StripeClient::activePricesFor()`s Vertrag wird ehrlich: er liefert aktive,
|
||||
wiederkehrende Preise **mit** ihren preisbestimmenden Eigenschaften; was davon
|
||||
zulässig ist, entscheidet der Aufrufer.
|
||||
|
||||
**Verwandt, hier mit erledigt:** `PlanPrices::ensure()` bekommt die
|
||||
`<= 0`-Schranke, die `AddonPrices::ensure()` schon hat. Sie 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.
|
||||
|
||||
## 4. `stripe:sweep-orphan-prices` — das Aufräum-Kommando
|
||||
|
||||
Der Vorgänger hat es bewusst ausgelassen (§12) und dafür die Log-Zeile
|
||||
begründet. Die Log-Zeile allein reicht nicht: sie nennt die Waise, aber niemand
|
||||
kann etwas mit ihr tun, ohne von Hand in Stripes Oberfläche zu gehen.
|
||||
|
||||
- **Ohne Argument:** ein Bericht. Für jedes unserer Produkte werden die aktiven
|
||||
Preise geholt und die aufgelistet, die **keine Zeile** in
|
||||
`stripe_addon_prices` oder `stripe_plan_prices` kennt — mit ID, Betrag,
|
||||
Währung, Intervall und Metadaten. Nichts wird angefasst.
|
||||
- **Mit `--archive`:** dieselbe Liste wird stillgelegt. Das ist harmlos, und
|
||||
zwar aus demselben Grund wie überall sonst in diesem Projekt: ein
|
||||
archivierter Preis verrechnet laufende Abos weiter, er wird nur nicht mehr
|
||||
verkauft.
|
||||
- **`--dry-run`** wie bei `stripe:sync-catalogue`, damit die beiden Kommandos
|
||||
sich gleich anfühlen.
|
||||
|
||||
Eine Waise, die der Wiedererkennungsschritt hätte übernehmen können, taucht hier
|
||||
gar nicht erst auf — sie wurde beim letzten Abgleich übernommen. Was übrig
|
||||
bleibt, ist genau das, was §12 als „nicht übernehmbar" beschrieben hat.
|
||||
|
||||
## 5. Die Warnung wird gedrosselt
|
||||
|
||||
`AdoptStripePrice` meldet einen unerklärlichen aktiven Preis mit
|
||||
`Log::warning` — bei **jedem** Abgleich und **jeder** Kundenbuchung, solange die
|
||||
Waise existiert. Der Widerspruchs-Pfad wurde aus genau diesem Grund still
|
||||
gestellt; der Bestätigungs-Pfad bekam dieselbe Überlegung nicht.
|
||||
|
||||
Höchstens einmal pro Preis-ID und Tag, über `Cache::add()` — dasselbe Muster wie
|
||||
`App\Livewire\Billing:377` und `App\Livewire\Admin\Vpn:329`. Die Drosselung gilt
|
||||
**nur** dieser wiederkehrenden Warnung, nicht der Meldung über ein doppeltes
|
||||
Produkt und nicht der über einen archivierten Doppel-Preis: die stehen für ein
|
||||
Ereignis, nicht für einen Zustand.
|
||||
|
||||
## 6. Die sechs mechanischen Punkte
|
||||
|
||||
Ohne Entwurfsentscheidung — bauen, Test, weiter (R22.5):
|
||||
|
||||
1. `FakeStripeClient::updatePriceMetadata()` **ersetzt**, wo Stripe
|
||||
**zusammenführt**. Bestehende Schlüssel bleiben künftig stehen.
|
||||
2. `FakeStripeClient::activePricesFor()` gibt Metadaten ungecastet zurück, wo
|
||||
`HttpStripeClient` auf String castet. Angleichen.
|
||||
3. `created` im Fake kann gleichstehen: `createPrice()` stempelt
|
||||
`count($prices) + 1`, `plantPrice()` hat die Vorgabe `1`. Gleichstand wird
|
||||
über die Preis-ID aufgelöst, damit die Wahl über Seitengrenzen hinweg
|
||||
reproduzierbar ist.
|
||||
4. `HttpStripeClient::activePricesFor()` bricht das Blättern **still** ab, wenn
|
||||
dem letzten Eintrag einer nicht-letzten Seite die `id` fehlt. Künftig wirft
|
||||
es. Begründung im Kommentar: bei `invoiceLines()` entsteht dadurch ein zu
|
||||
kurzer Belegtext, hier eine ungesehene Waise und ein zweiter aktiver Preis —
|
||||
also der Vorfall selbst.
|
||||
5. `stripe:sync-catalogue` zählt „angelegt" und „übernommen" **getrennt** statt
|
||||
die Wortwahl zu entschärfen.
|
||||
|
||||
Der naheliegende Weg funktioniert nicht: die Preis-ID vor und nach dem
|
||||
Aufruf zu vergleichen unterscheidet die beiden Fälle *nicht* — vorher gibt es
|
||||
in beiden keine, nachher in beiden eine. Stattdessen zählt
|
||||
`AdoptStripePrice` seine eigenen Übernahmen mit, und das Kommando liest den
|
||||
Zähler am Anfang und am Ende. Dafür wird die Klasse in `AppServiceProvider`
|
||||
als **Singleton** gebunden: sie wird heute je Aufruf frisch aufgelöst
|
||||
(`SyncStripeCatalogue` holt `PlanPrices` pro Zeile neu), ein Zähler auf der
|
||||
Instanz käme also nie über eins hinaus. Sie hat sonst keinen Zustand, und
|
||||
der Zähler ist das, was den Bericht wahr macht.
|
||||
6. `PlanPrices::ensure()` bekommt die `<= 0`-Schranke — siehe §3.
|
||||
|
||||
## 7. Was nicht dazugehört
|
||||
|
||||
- **Der sprunghafte `PlanCatalogueTest`.** Ursachensuche, keine Bauarbeit, und
|
||||
sie braucht ein anderes Vorgehen als dieser Plan.
|
||||
- **`CLUPILOT_DNS_ZONE` in `phpunit.xml` festnageln.** Fremder Fehler,
|
||||
inzwischen anders gelöst — die zweite Session hat die Zone im Test selbst
|
||||
festgenagelt (`a842512`).
|
||||
- **Alles, was laufende Verträge bewegt.** Wie im Vorgänger: dieser Entwurf
|
||||
bewegt keinen einzigen.
|
||||
|
||||
## 8. Tests
|
||||
|
||||
`tests/Feature/Billing/StripeProductAdoptionTest.php` (neu)
|
||||
|
||||
| Prüfung | Deckt ab |
|
||||
|---|---|
|
||||
| Verwaistes Produkt bei Stripe → wird übernommen, `createProduct` **nicht** aufgerufen (Familie **und** Modul) | §2 |
|
||||
| Produkt ohne unsere Metadaten → **nicht** übernommen, neues angelegt, Warnung | §2, Bedingung 2 |
|
||||
| Produkt, das einem Metadatenschlüssel widerspricht → nicht übernommen | §2, Bedingung 3 |
|
||||
| Zwei passende Produkte → ältestes übernommen, das andere **gemeldet und nicht stillgelegt** | §2 |
|
||||
|
||||
An `StripePriceAdoptionTest.php` angehängt:
|
||||
|
||||
| Prüfung | Deckt ab |
|
||||
|---|---|
|
||||
| Ein Preis mit `interval_count: 3` wird **von `AdoptStripePrice`** abgelehnt — über den Fake, nicht über `Http::fake` | §3 |
|
||||
| Dasselbe für `transform_quantity`, `usage_type: metered`, `billing_scheme: tiered` | §3 |
|
||||
| Zweite Warnung zur selben Preis-ID innerhalb eines Tages unterbleibt | §5 |
|
||||
| Gleichstand bei `created` → die kleinere Preis-ID gewinnt, zweimal gelaufen dasselbe Ergebnis | §6.3 |
|
||||
| Ein Lauf, der eine Waise übernimmt, meldet „übernommen" und **nicht** „angelegt" | §6.5 |
|
||||
|
||||
`tests/Feature/Billing/SweepOrphanPricesTest.php` (neu)
|
||||
|
||||
| Prüfung | Deckt ab |
|
||||
|---|---|
|
||||
| Bericht nennt die Waise und lässt sie in Ruhe | §4 |
|
||||
| `--archive` legt sie still, eine Zeile mit Preis bleibt unberührt | §4 |
|
||||
| `--dry-run` mit `--archive` legt nichts still | §4 |
|
||||
|
||||
Und an `StripeIdempotencyKeyTest.php`: Blättern **wirft** bei fehlender `id`
|
||||
(§6.4); der Fake führt Metadaten zusammen statt sie zu ersetzen (§6.1).
|
||||
|
||||
## 9. Berührte Dateien
|
||||
|
||||
**Neu**
|
||||
- `app/Services/Billing/AdoptStripeProduct.php`
|
||||
- `app/Console/Commands/SweepOrphanStripePrices.php`
|
||||
- `tests/Feature/Billing/StripeProductAdoptionTest.php`
|
||||
- `tests/Feature/Billing/SweepOrphanPricesTest.php`
|
||||
|
||||
**Geändert**
|
||||
- `app/Services/Stripe/StripeClient.php` — `activeProducts()`, Vertrag von
|
||||
`activePricesFor()`
|
||||
- `app/Services/Stripe/HttpStripeClient.php` — Produkte auflisten, Filter
|
||||
auflösen, Blättern wirft
|
||||
- `app/Services/Stripe/FakeStripeClient.php` — Produkte auflisten, vier Felder
|
||||
in `plantPrice()`, Metadaten zusammenführen, `created`-Gleichstand
|
||||
- `app/Services/Billing/AdoptStripePrice.php` — die vier Eigenschaften prüfen,
|
||||
Warnung drosseln
|
||||
- `app/Services/Billing/AddonPrices.php` — Produkt-Wiedererkennung in
|
||||
`product()`
|
||||
- `app/Services/Billing/PlanPrices.php` — `<= 0`-Schranke
|
||||
- `app/Console/Commands/SyncStripeCatalogue.php` — Produkt-Wiedererkennung,
|
||||
getrennte Zählung, Kommentar berichtigt
|
||||
- `tests/Feature/Billing/StripePriceAdoptionTest.php`,
|
||||
`StripeIdempotencyKeyTest.php` — angehängt
|
||||
- `docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md` — §14
|
||||
abhaken, was hier erledigt wird
|
||||
Loading…
Reference in New Issue