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>
This commit is contained in:
235
.paul/phases/119-reimport-total-paid-protection/119-01-PLAN.md
Normal file
235
.paul/phases/119-reimport-total-paid-protection/119-01-PLAN.md
Normal file
@@ -0,0 +1,235 @@
|
||||
---
|
||||
phase: 119-reimport-total-paid-protection
|
||||
plan: 01
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
files_modified:
|
||||
- src/Modules/Orders/OrderImportRepository.php
|
||||
- tests/Unit/Modules/Orders/OrderImportRepositoryTest.php
|
||||
- .paul/codebase/architecture.md
|
||||
- .paul/codebase/tech_changelog.md
|
||||
autonomous: true
|
||||
delegation: off
|
||||
---
|
||||
|
||||
<objective>
|
||||
## 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`.
|
||||
</objective>
|
||||
|
||||
<context>
|
||||
<clarifications>
|
||||
- **[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/`).
|
||||
</clarifications>
|
||||
|
||||
## 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`.
|
||||
</context>
|
||||
|
||||
<acceptance_criteria>
|
||||
|
||||
## AC-1: total_paid preserved when payment_status unchanged
|
||||
```gherkin
|
||||
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
|
||||
```gherkin
|
||||
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
|
||||
```gherkin
|
||||
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
|
||||
```gherkin
|
||||
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>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 1: Zmodyfikować updateOrderDelta() — conditional total_paid + is_canceled_by_buyer</name>
|
||||
<files>src/Modules/Orders/OrderImportRepository.php</files>
|
||||
<action>
|
||||
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).
|
||||
</action>
|
||||
<verify>
|
||||
`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.
|
||||
</verify>
|
||||
<done>AC-1, AC-2, AC-3 zaimplementowane.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Test jednostkowy OrderImportRepositoryTest</name>
|
||||
<files>tests/Unit/Modules/Orders/OrderImportRepositoryTest.php</files>
|
||||
<action>
|
||||
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 []/[].
|
||||
</action>
|
||||
<verify>
|
||||
`C:\xampp\php\php.exe vendor/bin/phpunit tests/Unit/Modules/Orders/OrderImportRepositoryTest.php`
|
||||
Wszystkie 3 nowe testy zielone.
|
||||
</verify>
|
||||
<done>AC-4 satisfied: test coverage dla AC-1, AC-2, AC-3.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: Update docs (architecture.md + tech_changelog.md)</name>
|
||||
<files>.paul/codebase/architecture.md, .paul/codebase/tech_changelog.md</files>
|
||||
<action>
|
||||
`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.
|
||||
</action>
|
||||
<verify>
|
||||
Pliki edytowane, nowe sekcje obecne. Czytanie ręczne.
|
||||
</verify>
|
||||
<done>Dokumentacja techniczna spójna z kodem.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<boundaries>
|
||||
|
||||
## 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.
|
||||
|
||||
</boundaries>
|
||||
|
||||
<verification>
|
||||
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
|
||||
</verification>
|
||||
|
||||
<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>
|
||||
|
||||
<output>
|
||||
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ć).
|
||||
</output>
|
||||
Reference in New Issue
Block a user