Files
orderPRO/.paul/phases/119-reimport-total-paid-protection/119-01-PLAN.md
Jacek Pyziak 3a2c419c25 feat(119): protect total_paid from re-import overwrite
OrderImportRepository::updateOrderDelta() przechodzi na dynamic SET builder.
total_paid jest dolaczane do UPDATE tylko gdy payment_status realnie sie
zmienia; is_canceled_by_buyer analogicznie, ale z override przez
cancelledBySource (cancel ze zrodla nadal propaguje sie do bazy).

Chroni reczne korekty operatora (zwroty czesciowe) przed cichym
nadpisaniem z payloadu zrodla przy kolejnym sync. Incydent #976:
operator zwrocil klientowi 28,00 PLN obnizajac total_paid 119->91,
co bez tej zmiany byloby cofniete przez kolejny re-import shoppro.

Boundaries: identical-payload guard, paymentTransition, statusOverwriteAllowed,
cancel propagation (status_code='anulowane') - bez zmian.

Tests: tests/Unit/OrderImportRepositoryTest.php - 3 scenariusze
(preserve / transition / cancel propagation) via Reflection + sqlite
in-memory. PHPUnit run odroczony (vendor/ gitignored).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-12 14:57:04 +02:00

14 KiB

phase, plan, type, wave, depends_on, files_modified, autonomous, delegation
phase plan type wave depends_on files_modified autonomous delegation
119-reimport-total-paid-protection 01 execute 1
src/Modules/Orders/OrderImportRepository.php
tests/Unit/Modules/Orders/OrderImportRepositoryTest.php
.paul/codebase/architecture.md
.paul/codebase/tech_changelog.md
true off
## Goal Zabezpieczenie pola `orders.total_paid` (i pomocniczo `is_canceled_by_buyer`) przed niepotrzebnym nadpisaniem z payloadu źródła podczas re-importu, gdy `payment_status` w bazie i z payloadu są identyczne. Pozwala to operatorowi ręcznie korygować `total_paid` (np. po częściowym zwrocie kasy) bez ryzyka, że kolejny sync nadpisze wartość z payloadu zewnętrznego, w którym kwota płatności pozostała niezmieniona.

Purpose

Realny incydent: zamówienie #976 — operator usunął 2 pozycje Girlanda i obniżył total_with_tax oraz total_paid ze 119,00 na 91,00 (zwrot 28,00 PLN do klienta). Audyt re-importu (Phase 112-01 delta-only) wykazał, że total_paid nie jest chronione: jeżeli shopPRO przyśle jakąkolwiek zmianę w payloadzie (np. status), updateOrderDelta() nadpisze total_paid = 119,00 z payloadu źródła. Eliminujemy tę regresję bez naruszania flow payment.status_changed (Phase 111) ani propagacji anulowania (Phase 112-01).

Output

  • Zmieniona logika OrderImportRepository::updateOrderDelta()total_paid i is_canceled_by_buyer aktualizowane tylko gdy payment_status realnie się zmienia (lub gdy źródło anuluje zamówienie).
  • Test jednostkowy w tests/Unit/Modules/Orders/OrderImportRepositoryTest.php pokrywający scenariusze: (a) payment_status stable → total_paid preserved, (b) payment_status transition → total_paid updated, (c) cancel propagation → is_canceled_by_buyer updated.
  • Aktualizacja .paul/codebase/architecture.md (sekcja Order Lifecycle → Re-import Phase 112) i .paul/codebase/tech_changelog.md.
- **[Warunek]** — Jakie pole(a) chronimy w `updateOrderDelta` gdy `payment_status` się nie zmienia? → Odpowiedź: Szerszy zestaw pól (total_paid + is_canceled_by_buyer; `status_code` już chronione przez istniejącą logikę preservacji z Phase 62; cancel propagation ze źródła musi nadal działać jako override). - **[Definicja]** — Co znaczy "payment_status się nie zmienia"? → Odpowiedź: `oldPaymentStatus === newPaymentStatus` (dowolna wartość: 0=0, 1=1, 2=2). - **[Backfill]** — Czy potrzebny skrypt CLI do wykrycia/poprawy "uszkodzonych" zamówień? → Odpowiedź: Nie — tylko fix forward. Order #976 poprawiony ręcznie, inne ad-hoc. - **[Test]** — Jak zweryfikować, że fix działa? → Odpowiedź: PHPUnit test (test jednostkowy w `tests/Unit/Modules/Orders/`).

Project Context

@.paul/PROJECT.md @.paul/ROADMAP.md @.paul/STATE.md @.paul/codebase/architecture.md @.paul/codebase/db_schema.md

Source Files

@src/Modules/Orders/OrderImportRepository.php

Prior Work

  • Phase 111 (payment.status_changed event) — re-import wykrywa paymentTransition 0/1→2.
  • Phase 112-01 (delta-only re-import) — updateOrderDelta() zawężony do minimalnego zestawu pól; replaceItems/Addresses/Notes tylko przy created=true.
  • Phase 56 (manual payments) — OrderPaymentsRepository aktualizuje total_paid przez recalculateTotalPaid(); operator może dodać/usunąć płatność ręcznie.

Key Constraint

Cancel propagation ze źródła (is_canceled_by_buyer=1 lub pull-mapped status_code='anulowane') musi nadal działać — istniejący override $cancelledBySource w upsertOrderAggregate() (linie 67-72) ustawia status_code='anulowane' niezależnie. Nasze zmiany w updateOrderDelta muszą propagować is_canceled_by_buyer=1 gdy źródło je flaguje, nawet przy stabilnym payment_status.

<acceptance_criteria>

AC-1: total_paid preserved when payment_status unchanged

Given zamówienie w DB ma `payment_status=2` i `total_paid=91.00` (po ręcznej korekcie operatora)
And payload ze źródła zawiera `payment_status=2` i kwoty wynikające z pełnego zamówienia (np. total_paid implikowane jako 119.00)
And payload różni się od zapisanego (np. inny `source_updated_at`) — identical-payload guard NIE zadziała
When `OrderImportRepository::upsertOrderAggregate()` wykonuje się dla tego zamówienia
Then po commit `orders.total_paid` pozostaje `91.00` (nie zmienia się)
And `orders.payment_status` pozostaje `2`
And `orders.source_updated_at`, `payload_json`, `fetched_at` aktualizują się normalnie

AC-2: total_paid updated on payment transition

Given zamówienie w DB ma `payment_status=0` i `total_paid=0.00`
And payload ze źródła zawiera `payment_status=2` i `total_paid=119.00` (potwierdzenie płatności)
When `upsertOrderAggregate()` wykonuje się
Then `orders.total_paid` zostaje zaktualizowane na `119.00`
And `orders.payment_status` zostaje zaktualizowane na `2`
And `paymentTransition=true` jest zwrócone (kompatybilność z Phase 111 chain rule #7)

AC-3: is_canceled_by_buyer propagated on source cancel

Given zamówienie w DB ma `payment_status=2`, `is_canceled_by_buyer=0`, `total_paid=91.00`
And payload ze źródła zawiera `payment_status=2`, `is_canceled_by_buyer=1` (klient anulował)
When `upsertOrderAggregate()` wykonuje się
Then `orders.is_canceled_by_buyer` zostaje zaktualizowane na `1`
And `orders.status_code` zostaje ustawione na `'anulowane'` (istniejąca cancel propagation)
And `orders.total_paid` pozostaje `91.00` (chroniony — payment_status nie zmienił się)

AC-4: Test coverage

Given test PHPUnit w `tests/Unit/Modules/Orders/OrderImportRepositoryTest.php`
When uruchomimy `php vendor/bin/phpunit tests/Unit/Modules/Orders/OrderImportRepositoryTest.php`
Then przejdą 3 nowe testy pokrywające AC-1, AC-2, AC-3
And istniejące testy `OrderImportRepository` (jeśli są) nie regredują

</acceptance_criteria>

Task 1: Zmodyfikować updateOrderDelta() — conditional total_paid + is_canceled_by_buyer src/Modules/Orders/OrderImportRepository.php W `upsertOrderAggregate()` (linia 89, przed wywołaniem `updateOrderDelta`): - Wyliczyć `$paymentStatusUnchanged = (int)($orderData['payment_status'] ?? 0) === (int)$oldPaymentStatus;` - Przekazać `$paymentStatusUnchanged` i `$cancelledBySource` do `updateOrderDelta()` jako nowe argumenty.
Zmodyfikować sygnaturę `updateOrderDelta()` (linia 194):
`private function updateOrderDelta(int $orderId, array $orderData, bool $paymentStatusUnchanged, bool $cancelledBySource): int`

Zmodyfikować SQL w `updateOrderDelta()`:
- Zbuduj listę pól do SET dynamicznie. Bazowa lista zawsze: `status_code, payment_status, source_updated_at, payload_json, fetched_at, updated_at = NOW()`.
- Jeżeli `!$paymentStatusUnchanged` → dodaj `total_paid = :total_paid`.
- Jeżeli `!$paymentStatusUnchanged || $cancelledBySource` → dodaj `is_canceled_by_buyer = :is_canceled_by_buyer`. (Cancel ze źródła wymusza update flagi nawet przy stabilnym payment_status.)
- Bind parametrów zgodnie z dynamiczną listą (tylko te które są w SET).

Zaktualizować doc-block nad metodą (Phase 119-01):
- Zaznaczyć, że `total_paid` jest aktualizowane tylko gdy `payment_status` się zmienia (chroni ręczne korekty kwoty).
- `is_canceled_by_buyer` aktualizowane gdy transition lub cancelledBySource.

Avoid:
- Pozostawienie static SQL z wszystkimi polami i NULL-owanie ich w bind — to nie chroni przed UPDATE (NULL nadpisze istniejącą wartość).
- Skomplikowanego query buildera; wystarczy tablica fragmentów `SET` łączona przez `implode(', ', ...)`.
- Łamania istniejącego identical-payload guard z linii 74-87 (pozostaje bez zmian).
`C:\xampp\php\php.exe -l src/Modules/Orders/OrderImportRepository.php` — syntax check passes. Czytanie kodu: w UPDATE SQL `total_paid` i `is_canceled_by_buyer` są warunkowe, pozostałe pola zawsze. AC-1, AC-2, AC-3 zaimplementowane. Task 2: Test jednostkowy OrderImportRepositoryTest tests/Unit/Modules/Orders/OrderImportRepositoryTest.php Utworzyć (lub rozszerzyć jeśli istnieje) plik testu PHPUnit dla `OrderImportRepository`.
Setup: in-memory SQLite z minimalnym schematem tabeli `orders` (kolumny używane przez `updateOrderDelta` + `insertOrder` na potrzeby testu) lub mock PDO. Preferowane: real PDO sqlite + CREATE TABLE w setUp() (analogicznie do innych testów w tests/Unit/).

Sprawdzić istniejące wzorce testów w `tests/Unit/Modules/Orders/` przed napisaniem (np. `OrderImportServiceTest.php` lub `AllegroOrderImportServiceTest.php` jeśli są).

Trzy scenariusze:

1. `testTotalPaidPreservedWhenPaymentStatusUnchanged()` (AC-1):
   - Pre-insert: `payment_status=2, total_paid=91.00, payload_json='{"a":1}'`.
   - Wywołaj `upsertOrderAggregate()` z payloadem `payment_status=2, total_paid=119.00, payload_json='{"a":2}'` (różny payload by uniknąć identical-guard).
   - Assert: row w DB ma `total_paid=91.00`, `payment_status=2`.

2. `testTotalPaidUpdatedOnPaymentTransition()` (AC-2):
   - Pre-insert: `payment_status=0, total_paid=0.00`.
   - Wywołaj z payloadem `payment_status=2, total_paid=119.00`.
   - Assert: `total_paid=119.00`, `payment_status=2`, return ma `payment_transition=true`.

3. `testIsCanceledByBuyerPropagatedOnSourceCancel()` (AC-3):
   - Pre-insert: `payment_status=2, total_paid=91.00, is_canceled_by_buyer=0`.
   - Wywołaj z payloadem `payment_status=2, total_paid=119.00, is_canceled_by_buyer=1`.
   - Assert: `is_canceled_by_buyer=1`, `status_code='anulowane'`, `total_paid=91.00` (still preserved).

Avoid:
- Mockowanie PDO statement-by-statement (kruche, trudne w utrzymaniu) — użyć real sqlite PDO.
- Testowania całego upsertOrderAggregate flow z addresses/items — minimalne arrays []/[].
`C:\xampp\php\php.exe vendor/bin/phpunit tests/Unit/Modules/Orders/OrderImportRepositoryTest.php` Wszystkie 3 nowe testy zielone. AC-4 satisfied: test coverage dla AC-1, AC-2, AC-3. Task 3: Update docs (architecture.md + tech_changelog.md) .paul/codebase/architecture.md, .paul/codebase/tech_changelog.md `architecture.md` — sekcja "Re-import (Phase 111 + 112)" pod "Order Lifecycle": - Dodać akapit "Phase 119-01 (total_paid protection)": gdy `paymentStatusUnchanged=true`, `updateOrderDelta()` nie aktualizuje `total_paid`. `is_canceled_by_buyer` aktualizowane tylko przy transition lub cancelledBySource. Chroni ręczne korekty kwot przez operatora.
`tech_changelog.md` — nowy wpis z datą `2026-05-12` (Phase 119-01):
- Krótki opis zmiany i powodu (incydent #976 — operator zwrócił 28,00 PLN klientowi, brak ochrony total_paid przed re-importem).
- Linki do plików: `OrderImportRepository.php`, test.

Nie powielać szczegółów implementacji — odsyłać do PLAN/SUMMARY.
Pliki edytowane, nowe sekcje obecne. Czytanie ręczne. Dokumentacja techniczna spójna z kodem.

DO NOT CHANGE

  • Logika identical-payload guard (normalizePayloadJson, linie 74-87) — pozostaje bez zmian, działa przed naszym warunkiem.
  • Logika paymentTransition i statusOverwriteAllowed (linie 60-65) — Phase 111 chain.
  • Logika cancel propagation (cancelledBySource, linie 67-72) — Phase 112-01 override.
  • replaceItems/replaceAddresses/replaceNotes/replacePayments/replaceShipments/replaceStatusHistory — wywoływane tylko przy created=true (Phase 112-01 delta-only contract).
  • Inne kolumny orders poza total_paid i is_canceled_by_buyer — kontrakt Phase 112-01 zostaje.

SCOPE LIMITS

  • Plan dotyczy WYŁĄCZNIE OrderImportRepository::updateOrderDelta() i jego wywołania w upsertOrderAggregate().
  • NIE backfill — istniejące zamówienia z potencjalnie nadpisanym total_paid nie są naprawiane skryptem (decyzja: fix forward).
  • NIE refaktor upsertOrderAggregate() na cleaner builder pattern (poza zakresem).
  • NIE zmiany w AllegroOrderImportService / ShopproOrdersSyncService — payment transition flow pozostaje.
Before declaring plan complete: - [ ] `C:\xampp\php\php.exe -l src/Modules/Orders/OrderImportRepository.php` — syntax OK - [ ] `C:\xampp\php\php.exe vendor/bin/phpunit tests/Unit/Modules/Orders/OrderImportRepositoryTest.php` — wszystkie nowe testy zielone - [ ] `C:\xampp\php\php.exe vendor/bin/phpunit tests/Unit/` — pełny suite bez regresji - [ ] Code review samego siebie: identical-payload guard wciąż działa pierwszy; cancel propagation wciąż override'uje status_code - [ ] AC-1, AC-2, AC-3, AC-4 spełnione

<success_criteria>

  • Wszystkie 3 taski zakończone, testy zielone.
  • Doc-block updateOrderDelta() jasno opisuje nowe zachowanie Phase 119.
  • .paul/codebase/architecture.md i tech_changelog.md zaktualizowane.
  • Brak regresji w istniejących testach (Phase 111 + 112 flow). </success_criteria>
After completion, create `.paul/phases/119-reimport-total-paid-protection/119-01-SUMMARY.md` describing: - Co zmieniono (lista plików + kluczowa zmiana w SQL builder). - Wyniki testów (3 nowe + suite count). - Open questions / follow-ups: czy podobna ochrona potrzebna dla `total_with_tax`/`delivery_price` (obecnie nie aktualizowane wcale przez `updateOrderDelta` — chronione od Phase 112-01, ale warto wzmiankować).