From 3a2c419c25898bb1b25d935c946e505d2e1f2e8f Mon Sep 17 00:00:00 2001 From: Jacek Pyziak Date: Tue, 12 May 2026 14:57:04 +0200 Subject: [PATCH] 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) --- .../119-01-PLAN.md | 235 ++++++++++++++++++ .../119-01-SUMMARY.md | 143 +++++++++++ src/Modules/Orders/OrderImportRepository.php | 52 ++-- tests/Unit/OrderImportRepositoryTest.php | 166 +++++++++++++ 4 files changed, 578 insertions(+), 18 deletions(-) create mode 100644 .paul/phases/119-reimport-total-paid-protection/119-01-PLAN.md create mode 100644 .paul/phases/119-reimport-total-paid-protection/119-01-SUMMARY.md create mode 100644 tests/Unit/OrderImportRepositoryTest.php diff --git a/.paul/phases/119-reimport-total-paid-protection/119-01-PLAN.md b/.paul/phases/119-reimport-total-paid-protection/119-01-PLAN.md new file mode 100644 index 0000000..36e3e0a --- /dev/null +++ b/.paul/phases/119-reimport-total-paid-protection/119-01-PLAN.md @@ -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 +--- + + +## 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`. + + + + +## 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ą +``` + + + + + + + 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 + + + +- 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). + + + +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ć). + diff --git a/.paul/phases/119-reimport-total-paid-protection/119-01-SUMMARY.md b/.paul/phases/119-reimport-total-paid-protection/119-01-SUMMARY.md new file mode 100644 index 0000000..c2d3eae --- /dev/null +++ b/.paul/phases/119-reimport-total-paid-protection/119-01-SUMMARY.md @@ -0,0 +1,143 @@ +--- +phase: 119-reimport-total-paid-protection +plan: 01 +subsystem: orders +tags: [reimport, idempotency, payment, refund, delta-update] + +requires: + - phase: 112-reimport-data-protection + provides: delta-only updateOrderDelta + identical-payload guard + - phase: 111-payment-transition-event + provides: paymentTransition detection (0/1 -> 2) + +provides: + - total_paid protection when payment_status unchanged + - is_canceled_by_buyer cancel-propagation override at field level + - dynamic SQL SET builder in updateOrderDelta + +affects: [orders re-import, manual payment corrections, refund flow, accounting] + +tech-stack: + added: [] + patterns: + - "Dynamic SET fragment builder for partial column updates (PHP array + implode)" + - "Boolean-flag-driven conditional column inclusion in UPDATE" + +key-files: + created: + - tests/Unit/OrderImportRepositoryTest.php + modified: + - src/Modules/Orders/OrderImportRepository.php + - .paul/codebase/architecture.md + - .paul/codebase/tech_changelog.md + +key-decisions: + - "total_paid chronione gdy oldPaymentStatus === newPaymentStatus (any value, not only 2->2)" + - "is_canceled_by_buyer chronione analogicznie, ale cancelledBySource override wymusza wpis" + - "Brak backfillu - fix forward (order #976 poprawiony recznie)" + - "Test flat path tests/Unit/ zamiast nested - match konwencji" + +patterns-established: + - "Dynamic SET builder: $setFragments[] + implode(', ', ...) zamiast statycznego SQL z NULL-bindem (NULL nadpisuje, NIE pomija)" + - "Conditional column inclusion poprzez flagi boolean wyliczane w upsertOrderAggregate i przekazywane do delta method" + +duration: ~20min +started: 2026-05-12T14:00:00Z +completed: 2026-05-12T14:20:00Z +--- + +# Phase 119 Plan 01: Re-import total_paid Protection Summary + +**`updateOrderDelta()` chroni `total_paid` (i pomocniczo `is_canceled_by_buyer`) przed nadpisaniem z payloadu zrodla gdy `payment_status` nie ulega zmianie - reczne korekty operatora (zwroty czesciowe) przezywaja re-import.** + +## Performance + +| Metric | Value | +|--------|-------| +| Duration | ~20 min | +| Started | 2026-05-12T14:00:00Z | +| Completed | 2026-05-12T14:20:00Z | +| Tasks | 3 completed | +| Files modified | 4 (1 source, 1 test, 2 docs) | + +## Acceptance Criteria Results + +| Criterion | Status | Notes | +|-----------|--------|-------| +| AC-1: total_paid preserved when payment_status unchanged | Pass (code + test) | SQL builder pomija `total_paid` gdy `$paymentStatusUnchanged=true`. Test napisany. | +| AC-2: total_paid updated on payment transition | Pass (code + test) | Gdy `payment_status` rozni sie (np. 0->2), fragment SET dolaczany. Test napisany. | +| AC-3: is_canceled_by_buyer propagated on source cancel | Pass (code + test) | Drugi warunek (`!$paymentStatusUnchanged || $cancelledBySource`) wymusza wpis flagi przy cancel ze zrodla nawet przy stabilnym `payment_status`. | +| AC-4: Test coverage | Partial | 3 testy napisane, syntax-checked (`php -l`). PHPUnit run odroczony - `vendor/` nieobecny, composer poza PATH. | + +## Accomplishments + +- Order #976 (i przyszle przypadki recznych korekt `total_paid`) chronione przed nadpisaniem przy kolejnym sync shoppro. +- `updateOrderDelta()` przeszedl ze static SQL na czytelny dynamic builder - latwo dodawac kolejne warunkowe kolumny w przyszlosci. +- Cancel propagation ze zrodla pozostaje silnym kontraktem - flaga `is_canceled_by_buyer` zawsze trafia do bazy gdy zrodlo anuluje, niezaleznie od ruchu `payment_status`. + +## Task Commits + +Atomic commits TBD (commit po UNIFY zgodnie z transition workflow). + +| Task | Type | Description | +|------|------|-------------| +| Task 1: Dynamic SET builder | feat | `updateOrderDelta()` warunkowe `total_paid` + `is_canceled_by_buyer` | +| Task 2: Test PHPUnit | test | 3 testy via Reflection + sqlite in-memory | +| Task 3: Docs update | docs | `architecture.md` + `tech_changelog.md` Phase 119-01 | + +## Files Created/Modified + +| File | Change | Purpose | +|------|--------|---------| +| `src/Modules/Orders/OrderImportRepository.php` | Modified | `upsertOrderAggregate` wylicza `$paymentStatusUnchanged`; `updateOrderDelta()` dynamic SET builder z warunkowym `total_paid` i `is_canceled_by_buyer` | +| `tests/Unit/OrderImportRepositoryTest.php` | Created | 3 testy PHPUnit pokrywajace AC-1/2/3 (sqlite + ReflectionMethod) | +| `.paul/codebase/architecture.md` | Modified | Sekcja Re-import rozszerzona o akapit Phase 119-01 | +| `.paul/codebase/tech_changelog.md` | Modified | Nowy wpis 2026-05-12 Phase 119 Plan 01 | + +## Decisions Made + +| Decision | Rationale | Impact | +|----------|-----------|--------| +| `paymentStatusUnchanged` definiowany jako `oldPaymentStatus === newPaymentStatus` (any value) | Konserwatywne ograniczenie do `2 == 2` byloby surprising; "any value" jest spojne z intencja "nie ruszaj gdy nic sie nie zmienilo" | Operator moze recznie ustawic `total_paid` przy dowolnym `payment_status` (np. czesciowa platnosc 1) i recznie ja korygowac | +| `is_canceled_by_buyer` chronione **ale** override przez `cancelledBySource` | Bez override blokowalibysmy propagacje anulowania ze zrodla - to bylby regres wzgledem Phase 112-01 | Cancel ze zrodla nadal flaguje zamowienie i ustawia `status_code='anulowane'` | +| Brak CLI backfillu | Order #976 poprawiony recznie; inne przypadki ad-hoc; plan maly i skupiony | Mozliwe stare zamowienia z "uszkodzonym" `total_paid` - dopuszczalne ryzyko | +| Test flat path (`tests/Unit/`) zamiast nested (`tests/Unit/Modules/Orders/`) | Match istniejacej konwencji projektu (wszystkie testy plasko) | Spojnosc struktury testow | + +## Deviations from Plan + +### Summary + +| Type | Count | Impact | +|------|-------|--------| +| Auto-fixed | 0 | - | +| Scope additions | 0 | - | +| Deferred | 1 | PHPUnit run odroczony do srodowiska z `composer install` | + +**Total impact:** Brak scope creep. Jedna deviation srodowiskowa (vendor/ gitignored). + +### Deferred Items + +- **PHPUnit run** - `vendor/` nieobecne, composer poza PATH. Test napisany i syntax-checked (`php -l`). Manual command po `composer install`: `vendor/bin/phpunit tests/Unit/OrderImportRepositoryTest.php`. Analogiczna sytuacja do gapow w Phase 116/117 (sonar-scanner). + +## Issues Encountered + +| Issue | Resolution | +|-------|------------| +| PHPUnit nieuruchamialny (brak vendor/) | Code + test napisany, syntax check przez `php -l`. Run odroczony. Pattern z Phase 117 (manual verification deferred). | +| Plan zakladal nested test path | Wybrano flat path - match konwencji projektu (wszystkie istniejace testy w `tests/Unit/` plasko, namespace `Tests\Unit`) | + +## Next Phase Readiness + +**Ready:** +- `OrderImportRepository::updateOrderDelta()` chroni reczne korekty `total_paid` przy kolejnych syncach. +- Pattern dynamic SET buildera dostepny dla przyszlych warunkowych UPDATE-ow. + +**Concerns:** +- Brak realnego potwierdzenia testem zielonym (vendor unavailable). Operator powinien uruchomic `composer install` + phpunit przed produkcyjnym push. +- Brak manualnego smoke testu na zywym shoppro - re-sync order #976 powinien potwierdzic, ze `total_paid=91.00` przetrwa. + +**Blockers:** None. + +--- +*Phase: 119-reimport-total-paid-protection, Plan: 01* +*Completed: 2026-05-12* diff --git a/src/Modules/Orders/OrderImportRepository.php b/src/Modules/Orders/OrderImportRepository.php index 143d67b..219297a 100644 --- a/src/Modules/Orders/OrderImportRepository.php +++ b/src/Modules/Orders/OrderImportRepository.php @@ -86,7 +86,8 @@ final class OrderImportRepository ]; } - $this->updateOrderDelta($orderId, $orderData); + $paymentStatusUnchanged = (int) ($orderData['payment_status'] ?? 0) === (int) $oldPaymentStatus; + $this->updateOrderDelta($orderId, $orderData, $paymentStatusUnchanged, $cancelledBySource); if ($paymentTransition || $statusOverwriteAllowed) { $this->replacePayments($orderId, $payments); @@ -189,33 +190,48 @@ final class OrderImportRepository * send_date_*, ordered_at, source_created_at, preferences_json, is_invoice, is_encrypted, * external_carrier_*, external_payment_type_id) are NOT overwritten on re-import. * + * Phase 119-01: When `payment_status` is unchanged between DB and source payload, + * `total_paid` and `is_canceled_by_buyer` are NOT included in the UPDATE. This protects + * manual operator corrections to `total_paid` (e.g. partial refunds) from being silently + * reverted to the source-side amount on the next sync. `is_canceled_by_buyer` is still + * forced into the UPDATE when `$cancelledBySource=true` so source-side cancellations + * propagate even if the payment status did not move. + * * @param array $orderData */ - private function updateOrderDelta(int $orderId, array $orderData): int + private function updateOrderDelta(int $orderId, array $orderData, bool $paymentStatusUnchanged, bool $cancelledBySource): int { - $statement = $this->pdo->prepare( - 'UPDATE orders - SET status_code = :status_code, - payment_status = :payment_status, - total_paid = :total_paid, - is_canceled_by_buyer = :is_canceled_by_buyer, - source_updated_at = :source_updated_at, - payload_json = :payload_json, - fetched_at = :fetched_at, - updated_at = NOW() - WHERE id = :id' - ); + $setFragments = [ + 'status_code = :status_code', + 'payment_status = :payment_status', + 'source_updated_at = :source_updated_at', + 'payload_json = :payload_json', + 'fetched_at = :fetched_at', + 'updated_at = NOW()', + ]; - $statement->execute([ + $params = [ 'id' => $orderId, 'status_code' => $orderData['status_code'] ?? null, 'payment_status' => $orderData['payment_status'] ?? null, - 'total_paid' => $orderData['total_paid'] ?? null, - 'is_canceled_by_buyer' => !empty($orderData['is_canceled_by_buyer']) ? 1 : 0, 'source_updated_at' => $orderData['source_updated_at'] ?? null, 'payload_json' => $this->encodeJson($orderData['payload_json'] ?? null), 'fetched_at' => $orderData['fetched_at'] ?? date('Y-m-d H:i:s'), - ]); + ]; + + if (!$paymentStatusUnchanged) { + $setFragments[] = 'total_paid = :total_paid'; + $params['total_paid'] = $orderData['total_paid'] ?? null; + } + + if (!$paymentStatusUnchanged || $cancelledBySource) { + $setFragments[] = 'is_canceled_by_buyer = :is_canceled_by_buyer'; + $params['is_canceled_by_buyer'] = !empty($orderData['is_canceled_by_buyer']) ? 1 : 0; + } + + $sql = 'UPDATE orders SET ' . implode(', ', $setFragments) . ' WHERE id = :id'; + $statement = $this->pdo->prepare($sql); + $statement->execute($params); return $orderId; } diff --git a/tests/Unit/OrderImportRepositoryTest.php b/tests/Unit/OrderImportRepositoryTest.php new file mode 100644 index 0000000..1703dc2 --- /dev/null +++ b/tests/Unit/OrderImportRepositoryTest.php @@ -0,0 +1,166 @@ +pdo = new PDO('sqlite::memory:'); + $this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + $this->pdo->sqliteCreateFunction('NOW', static fn(): string => date('Y-m-d H:i:s')); + + $this->pdo->exec( + 'CREATE TABLE orders ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + status_code TEXT, + payment_status INTEGER, + total_paid REAL, + is_canceled_by_buyer INTEGER DEFAULT 0, + source_updated_at TEXT, + payload_json TEXT, + fetched_at TEXT, + updated_at TEXT + )' + ); + + $this->repository = new OrderImportRepository($this->pdo); + } + + public function testTotalPaidPreservedWhenPaymentStatusUnchanged(): void + { + $this->seedOrder([ + 'status_code' => 'wyslane', + 'payment_status' => 2, + 'total_paid' => 91.00, + 'is_canceled_by_buyer' => 0, + 'payload_json' => '{"a":1}', + 'source_updated_at' => '2026-05-01 10:00:00', + 'fetched_at' => '2026-05-01 10:00:00', + ]); + + $orderData = [ + 'status_code' => 'wyslane', + 'payment_status' => 2, + 'total_paid' => 119.00, + 'is_canceled_by_buyer' => 0, + 'source_updated_at' => '2026-05-10 12:00:00', + 'payload_json' => ['a' => 2], + 'fetched_at' => '2026-05-10 12:00:00', + ]; + + $this->invokeUpdateOrderDelta(1, $orderData, true, false); + + $row = $this->fetchOrder(1); + self::assertSame('91', $this->normalizeAmount($row['total_paid']), 'total_paid must remain at manually corrected value'); + self::assertSame(2, (int) $row['payment_status']); + self::assertSame('2026-05-10 12:00:00', $row['source_updated_at']); + } + + public function testTotalPaidUpdatedOnPaymentTransition(): void + { + $this->seedOrder([ + 'status_code' => 'nieoplacone', + 'payment_status' => 0, + 'total_paid' => 0.00, + 'is_canceled_by_buyer' => 0, + 'payload_json' => '{}', + 'source_updated_at' => '2026-05-01 10:00:00', + 'fetched_at' => '2026-05-01 10:00:00', + ]); + + $orderData = [ + 'status_code' => 'w_realizacji', + 'payment_status' => 2, + 'total_paid' => 119.00, + 'is_canceled_by_buyer' => 0, + 'source_updated_at' => '2026-05-10 12:00:00', + 'payload_json' => ['a' => 1], + 'fetched_at' => '2026-05-10 12:00:00', + ]; + + $this->invokeUpdateOrderDelta(1, $orderData, false, false); + + $row = $this->fetchOrder(1); + self::assertSame('119', $this->normalizeAmount($row['total_paid'])); + self::assertSame(2, (int) $row['payment_status']); + } + + public function testIsCanceledByBuyerPropagatedOnSourceCancelEvenWhenPaymentStable(): void + { + $this->seedOrder([ + 'status_code' => 'wyslane', + 'payment_status' => 2, + 'total_paid' => 91.00, + 'is_canceled_by_buyer' => 0, + 'payload_json' => '{}', + 'source_updated_at' => '2026-05-01 10:00:00', + 'fetched_at' => '2026-05-01 10:00:00', + ]); + + $orderData = [ + 'status_code' => 'anulowane', + 'payment_status' => 2, + 'total_paid' => 119.00, + 'is_canceled_by_buyer' => 1, + 'source_updated_at' => '2026-05-10 12:00:00', + 'payload_json' => ['a' => 9], + 'fetched_at' => '2026-05-10 12:00:00', + ]; + + $this->invokeUpdateOrderDelta(1, $orderData, true, true); + + $row = $this->fetchOrder(1); + self::assertSame(1, (int) $row['is_canceled_by_buyer'], 'source cancellation must propagate'); + self::assertSame('anulowane', $row['status_code']); + self::assertSame('91', $this->normalizeAmount($row['total_paid']), 'total_paid still protected because payment_status unchanged'); + } + + /** + * @param array $orderData + */ + private function invokeUpdateOrderDelta(int $orderId, array $orderData, bool $paymentStatusUnchanged, bool $cancelledBySource): void + { + $method = new ReflectionMethod(OrderImportRepository::class, 'updateOrderDelta'); + $method->setAccessible(true); + $method->invoke($this->repository, $orderId, $orderData, $paymentStatusUnchanged, $cancelledBySource); + } + + /** + * @param array $values + */ + private function seedOrder(array $values): void + { + $columns = array_keys($values); + $placeholders = array_map(static fn(string $c): string => ':' . $c, $columns); + $sql = 'INSERT INTO orders (' . implode(', ', $columns) . ', updated_at) VALUES (' . implode(', ', $placeholders) . ", '2026-05-01 10:00:00')"; + $stmt = $this->pdo->prepare($sql); + $stmt->execute($values); + } + + /** + * @return array + */ + private function fetchOrder(int $id): array + { + $stmt = $this->pdo->prepare('SELECT * FROM orders WHERE id = :id'); + $stmt->execute(['id' => $id]); + $row = $stmt->fetch(PDO::FETCH_ASSOC); + self::assertIsArray($row); + return $row; + } + + private function normalizeAmount(mixed $value): string + { + return (string) (float) $value; + } +}