Warenwirtschaft: Review-Fix — API-Protokoll markiert verworfene Positionen korrekt
recordTransfer() löste die SKU erneut auf und markierte nur unbekannte Artikel als Fehler — Zeilen mit gültiger SKU aber fehlender Menge/Preis wurden verworfen und trotzdem grün protokolliert. prepareOrder() reicht jetzt included/product_id/product_name durch; failed = Position nicht übernommen. Entfernt zugleich die N+1-Produkt-Queries je Position. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
1b9b6adc44
commit
8ad0f65947
4 changed files with 50 additions and 8 deletions
|
|
@ -700,6 +700,12 @@ class ShoppingUserController extends Controller
|
|||
// $object = json_decode(json_encode($order), FALSE);
|
||||
$order = (object) $order;
|
||||
$error = [];
|
||||
// AP-13 (Review 20.07.2026): Zuordnung + Übernahme-Status hier mitliefern,
|
||||
// damit das Protokoll (ApiOrderLog::recordTransfer) exakt dieselbe Wahrheit
|
||||
// loggt und die SKU nicht erneut auflösen muss.
|
||||
$order->product_id = null;
|
||||
$order->product_name = null;
|
||||
$order->included = false;
|
||||
if (! isset($order->article) || ! isset($order->qty) || ! isset($order->price)) {
|
||||
$error[] = 'article parameter is missing';
|
||||
} else {
|
||||
|
|
@ -708,11 +714,14 @@ class ShoppingUserController extends Controller
|
|||
if (! $product) {
|
||||
$error[] = 'article not found';
|
||||
} else {
|
||||
$order->product_id = $product->id;
|
||||
$order->product_name = $product->name;
|
||||
if ($order->price != ($product->price * 100)) {
|
||||
$error[] = 'different price: '.($product->price * 100);
|
||||
}
|
||||
|
||||
Yard::instance('shopping')->add($product->id, $product->name, (int) $order->qty, $product->price, $product->tax, ['image' => [], 'slug' => $product->slug, 'weight' => $product->weight]);
|
||||
$order->included = true;
|
||||
}
|
||||
}
|
||||
$order->message = $error;
|
||||
|
|
|
|||
|
|
@ -59,9 +59,13 @@ class ApiOrderLog extends Model
|
|||
|
||||
/**
|
||||
* Erfolgreich verarbeitete Übertragung: Positionen inkl. Zuordnungsergebnis protokollieren.
|
||||
* Positionen ohne Produktzuordnung (SKU unbekannt / Parameter fehlen) markieren den Eintrag.
|
||||
* `failed` = Position wurde NICHT in die Bestellung übernommen (SKU unbekannt ODER
|
||||
* Menge/Preis fehlt) — die Wahrheit kommt direkt aus prepareOrder() (`included`,
|
||||
* `product_id`, `product_name`), damit Protokoll und Bestellung nie auseinanderlaufen
|
||||
* (Review 20.07.2026: vorher wurde die SKU hier erneut aufgelöst, wodurch Zeilen mit
|
||||
* fehlender Menge/Preis fälschlich als „OK" geloggt wurden — plus N+1-Queries).
|
||||
*
|
||||
* @param array<int, object> $lines Zeilen aus prepareOrder() (article/qty/price/message)
|
||||
* @param array<int, object> $lines Zeilen aus prepareOrder() (article/qty/price/message/included/product_id/product_name)
|
||||
*/
|
||||
public static function recordTransfer(mixed $wpOrderNumber, ShoppingUser $shoppingUser, ?ShoppingOrder $shoppingOrder, array $lines): self
|
||||
{
|
||||
|
|
@ -70,19 +74,17 @@ class ApiOrderLog extends Model
|
|||
|
||||
foreach ($lines as $line) {
|
||||
$line = (object) $line;
|
||||
$article = $line->article ?? null;
|
||||
$product = $article !== null ? Product::whereWpNumber($article)->first() : null;
|
||||
$failed = $product === null;
|
||||
$failed = ! (bool) ($line->included ?? false);
|
||||
if ($failed) {
|
||||
$failedCount++;
|
||||
}
|
||||
|
||||
$payloadLines[] = [
|
||||
'article' => $article,
|
||||
'article' => $line->article ?? null,
|
||||
'qty' => $line->qty ?? null,
|
||||
'price' => $line->price ?? null,
|
||||
'product_id' => $product?->id,
|
||||
'product_name' => $product?->name,
|
||||
'product_id' => $line->product_id ?? null,
|
||||
'product_name' => $line->product_name ?? null,
|
||||
'messages' => array_values((array) ($line->message ?? [])),
|
||||
'failed' => $failed,
|
||||
];
|
||||
|
|
|
|||
|
|
@ -277,6 +277,7 @@ Der Warenkorb kommt also **per Knopfdruck** (Fulfillment-Übergabe), nicht autom
|
|||
| 20.07.2026 | **Review-Fix: Stichtags-Schutz unveränderlich (`sale_stock_exempt`)** | Code-Review vor Live-Release: Der AP-13-Stichtags-Schutz hing an `shipped_at`, das `OrderStatusService::setShipped()` bei jedem Wechsel weg von versendet/abgeschlossen **nullt** und beim Zurückwechseln mit `now()` neu stempelt — Storno + erneutes Versenden einer Alt-Bestellung hätte rückwirkend vom heutigen Bestand abgezogen (Verletzung der dokumentierten Garantie, hinweise.md). Jetzt: Migration `2026_07_20_090000` ergänzt `shopping_orders.sale_stock_exempt` (bool) mit **einmaligem Backfill** (`shipped_at` < 16.07.2026 ⇒ exempt) — das Flag wird danach nie verändert; `syncSaleStock()` prüft nur noch das Flag, die Konstante `SALE_STOCK_SINCE` wandert in die Migration. | `SaleStockDeductionTest` +1: Toggle-Test storniert→versendet bucht nichts (12 grün) |
|
||||
| 20.07.2026 | **Review-Fix: Mahnwesen-Storno über zentralen Statuswechsel** | Code-Review vor Live-Release: Der Auto-Storno der Zahlungserinnerung (`set_order_status_cancelled`) schrieb `shipped = 10` direkt und rief nur `syncSaleStock()` — **vorbei am in AP-33a extrahierten `OrderStatusService::setShipped()`**. Folge: Bestand kam zurück, aber `margin->cancellation` blieb false, `partner_commission_pending_to`/`shipped_at` blieben gesetzt und die Promotions-/Guthaben-Folgen fehlten — die **Partnerprovision einer versendeten, nie bezahlten Bestellung wäre nach Ablauf der Pending-Frist zahlbar geworden** (Rechnungs-Bestellungen werden vor Zahlungseingang versendet; die Mahn-Abfrage filtert nicht auf Versandstatus). Jetzt ruft der Mahn-Storno `OrderStatusService::setShipped($id, 10)`. | `SaleStockDeductionTest` +1: Ende-zu-Ende über `sendReminder()` — Rückbuchung + Margen-Storno + pending_to/shipped_at genullt (13 grün) |
|
||||
| 20.07.2026 | **Review-Fix: AP-32-Brücke auf Base64 umgestellt (Connector 1.5.0)** | Code-Review vor Live-Release, drei Befunde in einem Zug behoben: **(1) Die Brücke hätte in Produktion nicht funktioniert** — der Connector 1.4.0 übertrug die rohe `label_url` aus `wp-content/uploads/woocommerce_dhl_label/`, den das DHL-Plugin per „deny from all" schützt → der Laravel-Download lief immer auf 403, das Label wurde nie gespeichert, und der View-Fallback verlinkte dieselbe 403-URL. **(2) SSRF:** der Server rief eine Client-URL ohne Host-/Schema-Prüfung ab. **(3) Timeout-Mismatch:** Download bis 20 s im Request-Pfad vs. 5 s WordPress-Default → Übertragung wäre shopseitig dauerhaft „fehlgeschlagen" (Exists-400 bei jedem Retry). Jetzt: Connector 1.5.0 liest die PDF vom Shop-Dateisystem (`label_path`, realpath-Pinning auf wp-uploads, `%PDF`-Check) und sendet sie **Base64** (`wp_label_pdf`); `storeWpLabel()` dekodiert + validiert (max. 10 MB, `%PDF`), **kein `Http::get` mehr**; `wp_label_url` bleibt reines Herkunfts-Metadatum, View-Fallback-Links entfernt; Connector-Timeout 30 s. Doku `ap-32-brueckenloesung-connector-update.md` + Hinweise überarbeitet (Deployment: Label kommt erst mit Connector 1.5.0 im Shop an). | `WpLabelTransferTest` auf Base64 umgestellt (10 grün; neu: ungültiges Base64/Nicht-PDF verworfen, URL-only = Altconnector blockiert nicht) |
|
||||
| 20.07.2026 | **Review-Fix: API-Protokoll — falsch-grüne Einträge behoben** | `ApiOrderLog::recordTransfer()` löste die SKU **erneut** auf und markierte nur unbekannte Artikel als Fehler — eine Zeile mit gültiger SKU, aber **fehlender Menge/Preis** wurde von `prepareOrder()` verworfen (Position fehlt in der Bestellung!) und trotzdem grün protokolliert. Jetzt liefert `prepareOrder()` je Zeile `included`/`product_id`/`product_name` mit und das Protokoll konsumiert nur noch diese Wahrheit — `failed` = „Position nicht übernommen" (deckt SKU unbekannt UND Parameter fehlen ab); nebenbei entfallen die N+1-Produkt-Queries je Position. | `ApiOrderLogTest` +1: bekannte SKU ohne Menge ⇒ rot markiert, Position fehlt in Bestellung (9 grün) |
|
||||
| _offen_ | AP-32 | Versand im System: DHL-Etiketten per API — **neu aufgenommen + Ansatz dokumentiert (§4, 15.07.2026)**; Klärungspunkte (DHL-Konto, Versandprodukte, Druck-Setup) für nächste Anforderungsrunde notiert. **Zwischenstand 17.07.2026: Brückenlösung aktiv** (Label aus dem Shop, s. Protokollzeile) — deckt den Versand-Flow ab, bis die DHL-Klärungen da sind | – |
|
||||
| _offen_ | AP-14/15/16/17 | Audit-Trail, Blockrechte, 2FA, WaWi-Einstellungen | – |
|
||||
|
||||
|
|
|
|||
|
|
@ -120,6 +120,36 @@ test('uebertragung mit unbekannter sku wird protokolliert und markiert', functio
|
|||
expect(ShoppingOrderItem::query()->where('shopping_order_id', $log->shopping_order_id)->count())->toBe(1);
|
||||
});
|
||||
|
||||
test('bekannte sku ohne menge wird als fehler markiert (position fehlt in der bestellung)', function () {
|
||||
// Review 20.07.2026: vorher hing `failed` nur am SKU-Lookup — eine Zeile mit
|
||||
// gültigem Artikel, aber fehlender Menge/Preis wurde verworfen und trotzdem
|
||||
// grün protokolliert.
|
||||
Mail::fake();
|
||||
Passport::actingAs(apilogMakeUser());
|
||||
$product = apilogMakeProduct('Apilog Öl', 300);
|
||||
apilogMakeProduct('Apilog Balsam', 301);
|
||||
apilogMakeShoppingUser(4715);
|
||||
|
||||
$this->postJson('https://api.'.config('app.api_domain').'/wp/order', [
|
||||
'wp_order_number' => 4715,
|
||||
'wp_order' => [
|
||||
['article' => 300, 'price' => 1990], // qty fehlt — wird nicht übernommen
|
||||
['article' => 301, 'qty' => 1, 'price' => 1990],
|
||||
],
|
||||
])->assertOk();
|
||||
|
||||
$log = ApiOrderLog::query()->sole();
|
||||
expect($log->has_errors)->toBeTrue()
|
||||
->and($log->error_summary)->toBe('1 von 2 Positionen ohne Produktzuordnung')
|
||||
->and($log->payload['lines'][0]['failed'])->toBeTrue()
|
||||
->and($log->payload['lines'][0]['messages'])->toContain('article parameter is missing')
|
||||
->and($log->payload['lines'][1]['failed'])->toBeFalse();
|
||||
|
||||
// Nur die vollständige Position landet in der Bestellung
|
||||
expect(ShoppingOrderItem::query()->where('shopping_order_id', $log->shopping_order_id)->count())->toBe(1)
|
||||
->and(ShoppingOrderItem::query()->where('product_id', $product->id)->count())->toBe(0);
|
||||
});
|
||||
|
||||
test('fehlerfreie uebertragung wird als ok protokolliert', function () {
|
||||
Mail::fake();
|
||||
Passport::actingAs(apilogMakeUser());
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue