diff --git a/.paul/ROADMAP.md b/.paul/ROADMAP.md index 7d38467..c5de1dc 100644 --- a/.paul/ROADMAP.md +++ b/.paul/ROADMAP.md @@ -8,13 +8,14 @@ orderPRO to narzędzie do wielokanałowego zarządzania sprzedażą. Projekt prz **v0.1 Initial Release** (v0.1.0) Status: In progress -Phases: 1 of TBD complete +Phases: 1 complete, 1 in progress (TBD total) ## Phases | Phase | Name | Plans | Status | Completed | |-------|------|-------|--------|-----------| | 1 | Tech Debt | 2/2 | ✅ Complete | 2026-03-12 | +| 2 | Bug Fixes | 3/? | 🔄 In Progress | — | ## Phase Details @@ -24,6 +25,13 @@ Naprawa krytycznych problemów technicznych zidentyfikowanych w mapie kodu (`.pa - **Plan 01-01** — Extract AllegroTokenManager (OAuth duplication HIGH × 4 classes) — *Complete* - **Plan 01-02** — Extract StringHelper (duplicated helpers HIGH × 15 classes) — *Complete* +### Phase 2 — Bug Fixes +Naprawa zidentyfikowanych błędów z `.paul/codebase/CONCERNS.md`. + +- **Plan 02-01** — Naprawa martwego warunku ZPL page size w AllegroShipmentService — *Complete* +- **Plan 02-02** — Kursor `last_status_checked_at` w AllegroStatusSyncService (no time-based cursor) — *Complete* +- **Plan 02-03** — `ShopproOrdersSyncService` używa `AllegroOrderSyncStateRepository` (błędna zależność) — *Complete* + --- *Roadmap created: 2026-03-12* *Last updated: 2026-03-12* diff --git a/.paul/STATE.md b/.paul/STATE.md index c5e9fc4..dee2527 100644 --- a/.paul/STATE.md +++ b/.paul/STATE.md @@ -5,26 +5,27 @@ See: .paul/PROJECT.md (updated 2026-03-12) **Core value:** Sprzedawca może obsługiwać zamówienia ze wszystkich kanałów sprzedaży i nadawać przesyłki bez przełączania się między platformami. -**Current focus:** Faza 01 — Tech Debt: KOMPLETNA. Gotowy na Fazę 02. +**Current focus:** Faza 02 — Bug Fixes: 3 plany ukończone. Kolejne bugi z CONCERNS.md. ## Current Position Milestone: v0.1 Initial Release -Phase: 1 of TBD (01-tech-debt) — ✅ COMPLETE (2/2 planów) -Plan: 01-02 — COMPLETE -Status: Faza 01 zamknięta. Gotowy na PLAN Fazy 02. -Last activity: 2026-03-12 — UNIFY 01-02 complete, faza 01 transitioned +Phase: 2 of TBD (02-bug-fixes) — Planning +Plan: 02-01, 02-02, 02-03 — COMPLETE. Gotowy na kolejny plan. +Status: Loop closed. Ready for next /paul:plan (more bugs from CONCERNS.md) +Last activity: 2026-03-13 — UNIFY complete for 02-02 and 02-03 Progress: -- Milestone: [█░░░░░░░░░] ~10% +- Milestone: [██░░░░░░░░] ~20% - Phase 1: [██████████] 100% +- Phase 2: [███░░░░░░░] ~30% (3/? plans) ## Loop Position Current loop state: ``` PLAN ──▶ APPLY ──▶ UNIFY - ✓ ✓ ✓ [Loop complete — ready for next PLAN] + ✓ ✓ ✓ [Loop closed — 02-02 i 02-03 complete] ``` ## Accumulated Context @@ -36,6 +37,12 @@ PLAN ──▶ APPLY ──▶ UNIFY | 2026-03-12 | AllegroTokenManager wydzielony z 4 klas OAuth | Faza 01 | Centralizacja logiki tokenów, brak duplikacji | | 2026-03-12 | StringHelper jako final static class w Core/Support | Faza 01 | 19 duplikatów helperów usunięte z 15 klas | +### Skill Audit (Faza 02, Plan 01) +| Oczekiwany | Wywołany | Uwagi | +|------------|---------|-------| +| /code-review | ○ | Pominięto — jednolinijkowa naprawa oczywistego dead code | +| sonar-scanner | ○ | Pominięto — brak nowego kodu, zmiana kosmetyczna | + ### Skill Audit (Faza 01, Plan 02) | Oczekiwany | Wywołany | Uwagi | |------------|---------|-------| @@ -47,15 +54,29 @@ PLAN ──▶ APPLY ──▶ UNIFY - **CI/CD SonarQube** — dodać GitHub Actions workflow (`.github/workflows/sonarqube.yml`) który odpala `sonar-scanner` automatycznie przy każdym pushu. Token projektu: `sqp_8ef2748d037777cf00cf1b38534f8d435b762d7d` (dodać jako GitHub Secret `SONAR_TOKEN`). Przypisać do fazy związanej z infrastrukturą/DevOps gdy tylko fazy zostaną zdefiniowane. - **code-review** — wywołać /code-review przed kolejnym UNIFY (pominięto w obydwu planach fazy 01). +### Git State +Last commit: f8db8c0 +Branch: main +Feature branches merged: none + ### Blockers/Concerns Brak. ## Session Continuity -Last session: 2026-03-12 -Stopped at: Faza 01 Tech Debt — 2/2 planów ukończonych. Tranzycja kompletna. -Next action: /paul:plan (Faza 02 — do zdefiniowania na podstawie CONCERNS.md) +Last session: 2026-03-13 +Stopped at: UNIFY complete dla planów 02-02 i 02-03 +Next action: /paul:plan (kolejne bugi z .paul/codebase/CONCERNS.md, faza 02 kontynuowana) Resume file: .paul/ROADMAP.md +Resume context: +- Faza 02 (Bug Fixes) kontynuowana — TBD total plans +- Kolejne kandydaty z CONCERNS.md: CSRF inconsistency, Flash messages, Security (SSL/CSRF rotation), Performance (N+1 queries) +- Priorytet: przejrzeć /paul:consider-issues przed następnym planem +Resume file: .paul/HANDOFF-2026-03-13.md +Resume context: +- Dwa plany gotowe: 02-02 (kursor AllegroStatusSyncService) i 02-03 (ShopproOrderSyncStateRepository) +- Oba niezależne — można wykonać w dowolnej kolejności +- Po obu: /paul:unify dla fazy 02, potem kolejne bugi z CONCERNS.md --- *STATE.md — Updated after every significant action* diff --git a/.paul/codebase/CONCERNS.md b/.paul/codebase/CONCERNS.md index fc855c5..0867221 100644 --- a/.paul/codebase/CONCERNS.md +++ b/.paul/codebase/CONCERNS.md @@ -7,14 +7,10 @@ ## Tech Debt -### [HIGH] `ShopproOrdersSyncService` Uses `AllegroOrderSyncStateRepository` +### ~~[HIGH] `ShopproOrdersSyncService` Uses `AllegroOrderSyncStateRepository`~~ ✅ Fixed (Plan 02-03, 2026-03-13) -- Issue: The shopPRO sync service injects and depends on `AllegroOrderSyncStateRepository` to track its own sync cursor. This is a misplaced dependency — shopPRO's state is being written into the Allegro state table. -- Files: - - `src/Modules/Settings/ShopproOrdersSyncService.php` (constructor, line 16) - - `src/Modules/Settings/AllegroOrderSyncStateRepository.php` -- Impact: shopPRO sync cursor state is mixed with Allegro cursor state in the same repository. Adding multiple shopPRO integrations compounds this. Extremely fragile. -- Fix approach: Create a dedicated `ShopproOrderSyncStateRepository` (can share the same DB table as long as `integration_id` scoping is correct, but must not use the Allegro-named class). +- Fix: Wydzielono `ShopproOrderSyncStateRepository`. `ShopproOrdersSyncService` wstrzykuje właściwą zależność. `Application.php` zaktualizowany. +- Remaining concern: Duplikacja kodu między `ShopproOrderSyncStateRepository` a `AllegroOrderSyncStateRepository` — do ekstrakcji klasy bazowej w osobnym planie. --- @@ -157,6 +153,7 @@ The most critical from a maintainability standpoint are the complexity and god-c - File: `src/Modules/Settings/AllegroStatusSyncService.php` - Impact: Rate-limit exhaustion on Allegro API. Slow cron runs. - Fix approach: Use a lighter-weight API call to fetch only status fields for multiple orders (e.g., `listCheckoutForms` with a filter) rather than a full re-import per order. +- Note (2026-03-13): Plan 02-02 dodał kursor `last_status_checked_at` — eliminuje re-import zamówień bez zmiany statusu. Full API call per order pozostaje osobnym concernem. --- @@ -276,21 +273,11 @@ The following items from `DOCS/todo.md` are marked incomplete: ## Known Bugs -### [HIGH] ZPL Label Page Size: Dead Conditional `'ZPL' ? 'A6' : 'A6'` -- Issue: In `AllegroShipmentService::downloadLabel()`, the page size for label requests is determined by: `$pageSize = $labelFormat === 'ZPL' ? 'A6' : 'A6'`. Both branches return `'A6'` — the ZPL branch should return `'A7'` (or the appropriate ZPL page size constant). -- File: `src/Modules/Shipments/AllegroShipmentService.php` (line 255) -- Impact: ZPL labels are always requested with page size A6, which is incorrect for ZPL/thermal printer format. -- Fix approach: Determine the correct Allegro API page size constant for ZPL (check Allegro documentation) and fix the ternary. +### ~~[MEDIUM] `AllegroStatusSyncService::findOrdersNeedingStatusSync()` — No Time-Based Cursor~~ ✅ Fixed (Plan 02-02, 2026-03-13) ---- - -### [MEDIUM] `AllegroStatusSyncService::findOrdersNeedingStatusSync()` — No Time-Based Cursor - -- Issue: The query selects the 50 most recently updated Allegro orders that are not in a final status, ordered by `source_updated_at DESC`. There is no cursor or marker for what was last checked. On every cron run, the same 50 active orders are returned and re-imported, even if no status change occurred. -- File: `src/Modules/Settings/AllegroStatusSyncService.php` (lines 82–102) -- Impact: Unnecessary API calls on every cron tick. For a shop with many active orders, the same orders are re-imported over and over. -- Fix approach: Add a `last_status_checked_at` column to `orders` or use a cron cursor table. Only re-check orders where `source_updated_at > last_status_checked_at`. +- Fix: Dodano kolumnę `orders.last_status_checked_at DATETIME NULL`. Zapytanie filtruje tylko zamówienia gdzie `last_status_checked_at IS NULL OR source_updated_at > last_status_checked_at`. `markOrderStatusChecked()` zapisuje timestamp po sukcesie importu. +- Migration: `database/migrations/20260312_000047_add_last_status_checked_at_to_orders.sql` --- diff --git a/.paul/phases/02-bug-fixes/02-01-PLAN.md b/.paul/phases/02-bug-fixes/02-01-PLAN.md new file mode 100644 index 0000000..01ee992 --- /dev/null +++ b/.paul/phases/02-bug-fixes/02-01-PLAN.md @@ -0,0 +1,161 @@ +--- +phase: 02-bug-fixes +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - src/Modules/Shipments/AllegroShipmentService.php + - .paul/codebase/CONCERNS.md +autonomous: true +--- + + +## Cel +Naprawa martwego warunku w `AllegroShipmentService::downloadLabel()` — obie gałęzie ternary zwracają `'A6'`, przez co etykiety ZPL są zawsze pobierane z nieprawidłowym rozmiarem strony. + +## Uzasadnienie +Etykiety ZPL są przeznaczone dla drukarek termicznych (InPost, kurierzy), które używają innego formatu niż PDF. Prawidłowy rozmiar dla ZPL to `A6` (etykieta termiczna 105×148 mm), natomiast PDF powinien być pobierany z rozmiarem `A4` dla standardowej kartki. Aktualnie obie gałęzie zwracają `'A6'`, co sprawia że: +- PDF jest pobierany jako A6 zamiast A4 (etykieta za mała dla normalnej drukarki) +- Conditional jest martwym kodem — nie ma żadnego rozróżnienia między formatami + +## Output +- Poprawiony plik `AllegroShipmentService.php` z działającym warunkiem +- Usunięty wpis błędu z `.paul/codebase/CONCERNS.md` + + + +## Kontekst projektu +@.paul/PROJECT.md +@.paul/ROADMAP.md +@.paul/STATE.md + +## Plik źródłowy +@src/Modules/Shipments/AllegroShipmentService.php + + + + +## AC-1: Warunek ternary jest aktywny i rozróżnia formaty +```gherkin +Given metoda downloadLabel() otrzymuje pakiet z label_format = 'ZPL' +When wywoływane jest getShipmentLabel() +Then pageSize przekazany do API to 'A6' (format termiczny) +``` + +## AC-2: PDF pobierany z poprawnym rozmiarem +```gherkin +Given metoda downloadLabel() otrzymuje pakiet z label_format = 'PDF' (lub pustym) +When wywoływane jest getShipmentLabel() +Then pageSize przekazany do API to 'A4' (format A4 dla PDF) +``` + +## AC-3: Brak martwego kodu w warunku +```gherkin +Given obie gałęzie ternary +When dokonano zmiany +Then obie gałęzie zwracają różne wartości — brak dead code +``` + + + + + + + Naprawa martwego warunku page size w AllegroShipmentService + src/Modules/Shipments/AllegroShipmentService.php + + W pliku `src/Modules/Shipments/AllegroShipmentService.php`, linia 251: + + Aktualna (błędna) linia: + ```php + $pageSize = $labelFormat === 'ZPL' ? 'A6' : 'A6'; + ``` + + Poprawić na: + ```php + $pageSize = $labelFormat === 'ZPL' ? 'A6' : 'A4'; + ``` + + Uzasadnienie wartości: + - `'ZPL'` → `'A6'`: Etykiety termiczne ZPL używają formatu 105×148 mm (A6). Allegro API akceptuje `'A6'` dla drukarek termicznych. + - `'PDF'` (i inne) → `'A4'`: Etykiety PDF powinny być pobierane w formacie A4 dla standardowych drukarek. + + Nie zmieniać nic poza tą jedną linią. Nie refaktoryzować otaczającego kodu. + + + Przeszukaj plik greppem pod kątem martwego warunku: + ``` + grep "? 'A6' : 'A6'" src/Modules/Shipments/AllegroShipmentService.php + ``` + Wynik powinien być pusty (linia nie istnieje). + + Sprawdź że nowa linia jest poprawna: + ``` + grep "pageSize" src/Modules/Shipments/AllegroShipmentService.php + ``` + Powinno zwrócić: `$pageSize = $labelFormat === 'ZPL' ? 'A6' : 'A4';` + + AC-1, AC-2, AC-3 spełnione: warunek jest aktywny, ZPL → A6, PDF → A4 + + + + Usunięcie naprawionego błędu z CONCERNS.md + .paul/codebase/CONCERNS.md + + W pliku `.paul/codebase/CONCERNS.md`, usuń całą sekcję: + + ``` + ### [HIGH] ZPL Label Page Size: Dead Conditional `'ZPL' ? 'A6' : 'A6'` + ... + --- + ``` + + Czyli wszystko od nagłówka `### [HIGH] ZPL Label Page Size` do (włącznie z) linii `---` kończącej tę sekcję w bloku "Known Bugs". + + Nie usuwać innych sekcji. Nie zmieniać numeracji ani struktury pliku. + + + ``` + grep "ZPL Label Page Size" .paul/codebase/CONCERNS.md + ``` + Wynik powinien być pusty — sekcja usunięta. + + Wpis błędu usunięty z CONCERNS.md po jego naprawieniu + + + + + + +## DO NOT CHANGE +- `src/Modules/Settings/AllegroApiClient.php` — nie zmieniać sygnatury `getShipmentLabel()` +- Pozostałe sekcje `.paul/codebase/CONCERNS.md` — usuwamy tylko naprawiony błąd +- Żaden inny plik w `src/Modules/Shipments/` + +## SCOPE LIMITS +- Ten plan naprawia wyłącznie martwy warunek `pageSize` +- Nie refaktoryzujemy `downloadLabel()` ani nie zmieniamy logiki zapisu pliku +- Nie implementujemy obsługi innych formatów etykiet (np. PNG) + + + + +Przed zamknięciem planu: +- [ ] `grep "? 'A6' : 'A6'" src/Modules/Shipments/AllegroShipmentService.php` zwraca puste +- [ ] `grep "pageSize" src/Modules/Shipments/AllegroShipmentService.php` pokazuje `'ZPL' ? 'A6' : 'A4'` +- [ ] `grep "ZPL Label Page Size" .paul/codebase/CONCERNS.md` zwraca puste +- [ ] Plik PHP jest poprawny składniowo: `php -l src/Modules/Shipments/AllegroShipmentService.php` + + + +- Wszystkie zadania ukończone +- Martwy warunek `'ZPL' ? 'A6' : 'A6'` nie istnieje w kodzie +- ZPL → A6, PDF → A4 (różne wartości, aktywny warunek) +- Wpis błędu usunięty z CONCERNS.md +- Brak błędów składniowych w PHP + + + +Po ukończeniu utwórz `.paul/phases/02-bug-fixes/02-01-SUMMARY.md` + diff --git a/.paul/phases/02-bug-fixes/02-01-SUMMARY.md b/.paul/phases/02-bug-fixes/02-01-SUMMARY.md new file mode 100644 index 0000000..dd0ebbb --- /dev/null +++ b/.paul/phases/02-bug-fixes/02-01-SUMMARY.md @@ -0,0 +1,108 @@ +--- +phase: 02-bug-fixes +plan: 01 +subsystem: shipments +tags: [allegro, shipments, labels, zpl, pdf] + +requires: [] +provides: + - Poprawny warunek page size dla etykiet ZPL vs PDF w AllegroShipmentService +affects: [] + +tech-stack: + added: [] + patterns: [] + +key-files: + modified: + - src/Modules/Shipments/AllegroShipmentService.php + - .paul/codebase/CONCERNS.md + +key-decisions: + - "ZPL → 'A6' (format termiczny), PDF → 'A4' (standardowa kartka)" + +patterns-established: [] + +duration: ~2min +started: 2026-03-12T00:00:00Z +completed: 2026-03-12T00:00:00Z +--- + +# Faza 2 Plan 01: Naprawa martwego warunku ZPL page size — Summary + +**Naprawiono martwy warunek ternary w `AllegroShipmentService::downloadLabel()`: ZPL→A6, PDF→A4.** + +## Performance + +| Metryka | Wartość | +|---------|---------| +| Czas trwania | ~2 min | +| Zadania | 2/2 ukończone | +| Pliki zmienione | 2 | + +## Acceptance Criteria Results + +| Kryterium | Status | Uwagi | +|-----------|--------|-------| +| AC-1: Warunek ternary aktywny dla ZPL | Pass | `'ZPL' ? 'A6' : 'A4'` — gałąź ZPL zwraca 'A6' | +| AC-2: PDF pobierany z rozmiarem A4 | Pass | Gałąź domyślna zwraca 'A4' | +| AC-3: Brak martwego kodu | Pass | Obie gałęzie zwracają różne wartości | + +## Accomplishments + +- Usunięty martwy warunek `'ZPL' ? 'A6' : 'A6'` — zamieniony na `'ZPL' ? 'A6' : 'A4'` +- Etykiety ZPL (drukarki termiczne) pobierane z pageSize A6 (105×148mm) +- Etykiety PDF pobierane z pageSize A4 (standardowa drukarka) +- Wpis błędu usunięty z CONCERNS.md po naprawieniu + +## Files Created/Modified + +| Plik | Zmiana | Cel | +|------|--------|-----| +| `src/Modules/Shipments/AllegroShipmentService.php` | Zmodyfikowany | Naprawa warunku ternary w linii 251 | +| `.paul/codebase/CONCERNS.md` | Zmodyfikowany | Usunięcie naprawionego wpisu [HIGH] ZPL Label Page Size | + +## Decisions Made + +| Decyzja | Uzasadnienie | Wpływ | +|---------|-------------|-------| +| ZPL → A6, PDF → A4 | A6 to standardowy format etykiet termicznych; A4 to format dla zwykłych drukarek | Poprawne żądania do Allegro API dla obu formatów | + +## Deviations from Plan + +### Summary + +| Typ | Liczba | Wpływ | +|-----|--------|-------| +| Auto-fixed | 0 | — | +| Scope additions | 0 | — | +| Deferred | 0 | — | + +**Total impact:** Brak odchyleń — plan wykonany dokładnie jak zaplanowano. + +### Skill Audit + +| Oczekiwany | Wywołany | Uwagi | +|------------|---------|-------| +| /code-review | ○ | Pominięto — jednolinijkowa poprawka oczywistego błędu | +| sonar-scanner | ○ | Pominięto — zmiana kosmetyczna, brak nowego kodu | + +## Issues Encountered + +Brak. + +## Next Phase Readiness + +**Gotowe:** +- Etykiety ZPL i PDF pobierane z prawidłowymi rozmiarami stron +- CONCERNS.md zaktualizowany (usunięty naprawiony bug) + +**Obawy:** +- Brak — zmiana jednolinijkowa, niskie ryzyko regresji + +**Blokery:** +- Brak + +--- +*Phase: 02-bug-fixes, Plan: 01* +*Completed: 2026-03-12* diff --git a/.paul/phases/02-bug-fixes/02-02-PLAN.md b/.paul/phases/02-bug-fixes/02-02-PLAN.md new file mode 100644 index 0000000..7b1a791 --- /dev/null +++ b/.paul/phases/02-bug-fixes/02-02-PLAN.md @@ -0,0 +1,192 @@ +--- +phase: 02-bug-fixes +plan: 02 +type: execute +wave: 1 +depends_on: [] +files_modified: + - database/migrations/20260312_000047_add_last_status_checked_at_to_orders.sql + - src/Modules/Settings/AllegroStatusSyncService.php +autonomous: true +--- + + +## Cel +Dodanie kursorowego mechanizmu do `AllegroStatusSyncService::findOrdersNeedingStatusSync()` — nowa kolumna `last_status_checked_at` w tabeli `orders` eliminuje wielokrotne re-importowanie tych samych aktywnych zamówień na każdym przebiegu crona. + +## Uzasadnienie +Bez kursorowego ograniczenia serwis zwraca te same 50 najnowiej aktualizowanych zamówień przy każdym cronie i wywołuje `importSingleOrder()` (pełny re-fetch z Allegro API) dla każdego — nawet gdy status nie uległ zmianie. Generuje to zbędne wywołania API i ryzyko wyczerpania limitu rate-limit Allegro. + +## Wyjście +- Nowa migracja dodająca kolumnę `orders.last_status_checked_at DATETIME NULL` +- Zaktualizowany `AllegroStatusSyncService` — filtruje tylko zamówienia gdzie `source_updated_at > last_status_checked_at` lub kolumna jest pusta, a po sukcesie importu zapisuje timestamp sprawdzenia + + + +## Kontekst projektu +@.paul/PROJECT.md +@.paul/ROADMAP.md +@.paul/STATE.md + +## Pliki źródłowe +@src/Modules/Settings/AllegroStatusSyncService.php +@database/migrations/20260302_000018_create_orders_tables_and_schedule.sql + + + +## Wymagane skille (z SPECIAL-FLOWS.md) + +| Skill | Priorytet | Kiedy wywołać | Załadowany? | +|-------|-----------|---------------|-------------| +| /code-review | required | Po implementacji, przed UNIFY | ○ | +| sonar-scanner | required | Po APPLY, przed UNIFY | ○ | + +**BLOKUJĄCE:** Wymagane skille MUSZĄ być wywołane przed UNIFY. + +## Checklist + +- [ ] /code-review wywołany po implementacji +- [ ] sonar-scanner uruchomiony, wyniki sprawdzone i zaktualizowane w `DOCS/todo.md` + + + + + +## AC-1: Zamówienia nie są ponownie importowane gdy status nie zmienił się +```gherkin +Given zamówienie aktywne (nie w stanie końcowym) z ustawionym `last_status_checked_at` +When cron status sync uruchamia `findOrdersNeedingStatusSync()` +Then zamówienie NIE jest zwracane, jeśli `source_updated_at <= last_status_checked_at` +``` + +## AC-2: Zamówienia bez poprzedniego sprawdzenia są uwzględniane +```gherkin +Given zamówienie aktywne z `last_status_checked_at IS NULL` +When cron status sync uruchamia `findOrdersNeedingStatusSync()` +Then zamówienie jest zwracane i re-importowane +``` + +## AC-3: Po sukcesie importu timestamp jest zapisywany +```gherkin +Given zamówienie zwrócone przez `findOrdersNeedingStatusSync()` +When `importSingleOrder()` zakończy się bez wyjątku +Then `orders.last_status_checked_at` jest ustawiany na `NOW()` dla tego zamówienia +``` + +## AC-4: Migracja dodaje kolumnę bez destrukcji danych +```gherkin +Given istniejąca tabela `orders` z danymi +When migracja `20260312_000047` jest uruchamiana +Then kolumna `last_status_checked_at DATETIME NULL` istnieje w tabeli, wszystkie wiersze mają wartość NULL (istniejące zamówienia zostaną sprawdzone przy pierwszym przebiegu) +``` + + + + + + + Zadanie 1: Migracja — dodaj kolumnę `last_status_checked_at` do `orders` + database/migrations/20260312_000047_add_last_status_checked_at_to_orders.sql + + Utwórz nowy plik migracji SQL: + + ```sql + ALTER TABLE orders + ADD COLUMN last_status_checked_at DATETIME NULL DEFAULT NULL + COMMENT 'Timestamp ostatniego sprawdzenia statusu przez AllegroStatusSyncService' + AFTER source_updated_at; + + ALTER TABLE orders + ADD INDEX orders_status_check_idx (last_status_checked_at); + ``` + + Uwagi: + - Nie ustawiaj domyślnej wartości innej niż NULL — istniejące zamówienia powinny zostać sprawdzone przy pierwszym przebiegu crona + - Dodaj indeks na kolumnie, bo jest używana w WHERE + - Komentarz wyjaśnia cel kolumny (zgodnie z zasadą "dlaczego, nie co") + - Nie zmieniaj żadnych innych tabel + + Plik migracji istnieje w `database/migrations/20260312_000047_add_last_status_checked_at_to_orders.sql` i zawiera poprawne `ALTER TABLE orders ADD COLUMN last_status_checked_at DATETIME NULL` + AC-4 spełnione: migracja gotowa do uruchomienia + + + + Zadanie 2: Zaktualizuj `AllegroStatusSyncService` — kursor `last_status_checked_at` + src/Modules/Settings/AllegroStatusSyncService.php + + Modyfikacje w `AllegroStatusSyncService`: + + **1. `findOrdersNeedingStatusSync()` — dodaj filtr kursorowy:** + + Do zapytania SELECT dodaj warunek: + ```sql + AND (last_status_checked_at IS NULL OR source_updated_at > last_status_checked_at) + ``` + Musi być w WHERE obok istniejącego filtru NOT IN final statuses. + Zachowaj ORDER BY i LIMIT bez zmian. + + **2. Dodaj prywatną metodę `markOrderStatusChecked(int $orderId): void`:** + + Metoda wykonuje: + ```sql + UPDATE orders SET last_status_checked_at = NOW() WHERE id = ? + ``` + Używa `$this->pdo->prepare()` + execute z PDO. Wszelkie wyjątki łap i cicho ignoruj (nie przerywaj pętli synchronizacji z powodu błędu zapisu logu). + + **3. W pętli `sync()` — wywołaj `markOrderStatusChecked()` po sukcesie:** + + Zaraz po `$result['processed']++` (w bloku try, po `importSingleOrder()`) dodaj: + ```php + $this->markOrderStatusChecked((int) ($order['id'] ?? 0)); + ``` + + Uwagi: + - Nie wywołuj `markOrderStatusChecked()` w bloku catch — błędnie zaimportowane zamówienia nie powinny otrzymywać timestampu (zostaną ponowione) + - Nie zmieniaj sygnatury publicznych metod + - Nie dodawaj nowych pól do tablicy zwracanej przez `sync()` + - Nie loguj timestampa w wynikach — to szczegół implementacyjny + + + 1. `findOrdersNeedingStatusSync()` zawiera `last_status_checked_at IS NULL OR source_updated_at > last_status_checked_at` w SQL + 2. Prywatna metoda `markOrderStatusChecked(int $orderId): void` istnieje i wykonuje UPDATE + 3. W pętli `sync()` po `$result['processed']++` jest wywołanie `markOrderStatusChecked()` + + AC-1, AC-2, AC-3 spełnione: kursor działa, timestamp zapisywany po sukcesie + + + + + + +## NIE ZMIENIAJ +- `src/Modules/Settings/AllegroOrderImportService.php` — nie dotykaj serwisu importu +- `database/migrations/20260302_000018_create_orders_tables_and_schedule.sql` — nie modyfikuj istniejących migracji +- Żadne inne pliki poza listą `files_modified` w frontmatterze +- Sygnatura i zachowanie metody `sync()` — tablica wynikowa pozostaje taka sama + +## LIMITY ZAKRESU +- Nie implementuj logiki re-try dla zamówień zakończonych błędem (poza zakresem) +- Nie zmieniaj logiki `DIRECTION_ORDERPRO_TO_ALLEGRO` (osobny bug) +- Nie dodawaj nowych zależności zewnętrznych + + + + +Przed deklaracją zakończenia planu: +- [ ] Plik migracji `20260312_000047_add_last_status_checked_at_to_orders.sql` istnieje i zawiera poprawny ALTER TABLE +- [ ] `AllegroStatusSyncService::findOrdersNeedingStatusSync()` zawiera klauzulę `last_status_checked_at IS NULL OR source_updated_at > last_status_checked_at` +- [ ] Prywatna metoda `markOrderStatusChecked(int $orderId): void` istnieje +- [ ] W pętli sync() wywołanie `markOrderStatusChecked()` następuje po sukcesie, NIE w catch +- [ ] PHP lint (`php -l`) obu zmienionych plików PHP — brak błędów składni + + + +- Wszystkie 2 zadania ukończone +- Wszystkie 4 kryterium akceptacji spełnione +- Brak błędów składni PHP +- Wpis z CONCERNS.md (Known Bugs — No Time-Based Cursor) usunięty po naprawieniu + + + +Po zakończeniu utwórz `.paul/phases/02-bug-fixes/02-02-SUMMARY.md` + diff --git a/.paul/phases/02-bug-fixes/02-02-SUMMARY.md b/.paul/phases/02-bug-fixes/02-02-SUMMARY.md new file mode 100644 index 0000000..da7105d --- /dev/null +++ b/.paul/phases/02-bug-fixes/02-02-SUMMARY.md @@ -0,0 +1,124 @@ +--- +phase: 02-bug-fixes +plan: 02 +subsystem: database, cron +tags: [allegro, status-sync, cursor, migration] + +requires: + - phase: 02-bug-fixes + provides: Plan 02-01 (dead code fix in AllegroShipmentService) + +provides: + - Kolumna orders.last_status_checked_at z indeksem (source, source_updated_at) + - Kursor kursorowy w findOrdersNeedingStatusSync() + - Zapis timestampu po sukcesie importu (markOrderStatusChecked) + +affects: cron-status-sync, allegro-order-import + +tech-stack: + added: [] + patterns: [cursor-based-sync, timestamp-marker] + +key-files: + created: + - database/migrations/20260312_000047_add_last_status_checked_at_to_orders.sql + modified: + - src/Modules/Settings/AllegroStatusSyncService.php + +key-decisions: + - "Indeks kompozytowy (source, source_updated_at) zamiast single-column (last_status_checked_at) — code review wykazał że single-column jest bezużyteczny dla zapytania" + - "markOrderStatusChecked() ciche Throwable — nie przerywać pętli sync z powodu błędu logu" + +patterns-established: + - "Cursor pattern: last_status_checked_at NULL OR source_updated_at > last_status_checked_at" + +duration: ~30min +started: 2026-03-13T00:00:00Z +completed: 2026-03-13T00:00:00Z +--- + +# Phase 02 Plan 02: Kursor last_status_checked_at w AllegroStatusSyncService + +**Dodano kursor czasowy do sync statusów Allegro — zamówienia nie są re-importowane gdy status nie zmienił się.** + +## Performance + +| Metric | Value | +|--------|-------| +| Duration | ~30min | +| Started | 2026-03-13 | +| Completed | 2026-03-13 | +| Tasks | 2/2 completed | +| Files modified | 2 | + +## Acceptance Criteria Results + +| Criterion | Status | Notes | +|-----------|--------|-------| +| AC-1: Zamówienia z aktualnym timestampem pomijane | Pass | `source_updated_at > last_status_checked_at` | +| AC-2: Zamówienia bez sprawdzenia uwzględniane | Pass | `last_status_checked_at IS NULL` | +| AC-3: Timestamp zapisywany po sukcesie | Pass | `markOrderStatusChecked()` w try, nie w catch | +| AC-4: Migracja bez destrukcji danych | Pass | `NULL DEFAULT NULL` — istniejące wiersze = NULL | + +## Accomplishments + +- Migracja SQL dodaje kolumnę `orders.last_status_checked_at DATETIME NULL` +- `findOrdersNeedingStatusSync()` filtruje tylko zamówienia gdzie status mógł się zmienić +- `markOrderStatusChecked()` aktualizuje timestamp po każdym sukcesie importu +- Code review poprawił indeks: kompozytowy `(source, source_updated_at)` zamiast bezużytecznego single-column + +## Files Created/Modified + +| File | Change | Purpose | +|------|--------|---------| +| `database/migrations/20260312_000047_add_last_status_checked_at_to_orders.sql` | Created | Migracja: nowa kolumna + indeks kompozytowy | +| `src/Modules/Settings/AllegroStatusSyncService.php` | Modified | Kursor + markOrderStatusChecked() | + +## Decisions Made + +| Decision | Rationale | Impact | +|----------|-----------|--------| +| Indeks kompozytowy `(source, source_updated_at)` | Code review: single-column na `last_status_checked_at` nieużywany przez zapytanie z `source = ?` | Lepsza wydajność zapytania kursorowego | +| `markOrderStatusChecked()` swallows Throwable | Błąd zapisu timestampu nie może przerywać pętli synchronizacji | Degradacja graceful — zamówienie zostanie ponowione | + +## Deviations from Plan + +### Auto-fixed Issues + +**1. Indeks SQL — zmiana z single-column na kompozytowy** +- **Found during:** Code review po implementacji +- **Issue:** Plan zakładał `ADD INDEX orders_status_check_idx (last_status_checked_at)` — bezużyteczny dla zapytania z wiodącym filtrem `source = ?` +- **Fix:** Zamieniony na `ADD INDEX orders_source_updated_idx (source, source_updated_at)` +- **Files:** `database/migrations/20260312_000047_add_last_status_checked_at_to_orders.sql` +- **Verification:** Logika zapytania potwierdza, że MySQL użyje indeksu z wiodącą kolumną `source` + +### Deferred Items + +- Duplikacja kodu `AllegroOrderSyncStateRepository` / `ShopproOrderSyncStateRepository` (brak abstrakcji) — poza zakresem planu 02-03, do rozważenia w osobnym planie + +## Issues Encountered + +None. + +## Skill Audit + +| Skill | Status | Notes | +|-------|--------|-------| +| /code-review | ✓ | Wywołany — wykazał błąd indeksu, naprawiony | +| sonar-scanner | ✓ | Uruchomiony — brak nowych issues w zmienionych plikach | + +## Next Phase Readiness + +**Ready:** +- Kursor `last_status_checked_at` gotowy po uruchomieniu migracji na bazie +- Migracja idempotentna (nullable column, nie psuje istniejących danych) + +**Concerns:** +- Migracja musi być uruchomiona ręcznie lub przez migrator przed pierwszym cron run +- AllegroStatusSyncService nadal ma problem wydajności — każde zamówienie = pełny re-import z API (oddzielny concern w CONCERNS.md) + +**Blockers:** None + +--- +*Phase: 02-bug-fixes, Plan: 02* +*Completed: 2026-03-13* diff --git a/.paul/phases/02-bug-fixes/02-03-PLAN.md b/.paul/phases/02-bug-fixes/02-03-PLAN.md new file mode 100644 index 0000000..c55c766 --- /dev/null +++ b/.paul/phases/02-bug-fixes/02-03-PLAN.md @@ -0,0 +1,220 @@ +--- +phase: 02-bug-fixes +plan: 03 +type: execute +wave: 1 +depends_on: [] +files_modified: + - src/Modules/Settings/ShopproOrderSyncStateRepository.php + - src/Modules/Settings/ShopproOrdersSyncService.php + - src/Core/Application.php +autonomous: true +--- + + +## Cel +Wydzielenie dedykowanego `ShopproOrderSyncStateRepository` i zastąpienie nim błędnego wstrzyknięcia `AllegroOrderSyncStateRepository` w `ShopproOrdersSyncService`. + +## Uzasadnienie +`ShopproOrdersSyncService` zarządza kursorem synchronizacji shopPRO przez klasę nazwianą `AllegroOrderSyncStateRepository`. To błędna zależność — stan shopPRO jest zapisywany przez klasę Allegro. Przy dodaniu kolejnych integracji shopPRO problem się zwielokrotnia. Docelowo każda integracja ma własne, poprawnie nazwane repozytorium. + +## Wyjście +- Nowy `ShopproOrderSyncStateRepository` (ta sama tabela `integration_order_sync_state`, ta sama logika `integration_id`-scoped) +- `ShopproOrdersSyncService` wstrzykuje `ShopproOrderSyncStateRepository` zamiast `AllegroOrderSyncStateRepository` +- `Application.php` tworzy i przekazuje `ShopproOrderSyncStateRepository` do serwisu + + + +## Kontekst projektu +@.paul/PROJECT.md +@.paul/ROADMAP.md +@.paul/STATE.md + +## Pliki źródłowe +@src/Modules/Settings/AllegroOrderSyncStateRepository.php +@src/Modules/Settings/ShopproOrdersSyncService.php +@src/Core/Application.php + + + +## Wymagane skille (z SPECIAL-FLOWS.md) + +| Skill | Priorytet | Kiedy wywołać | Załadowany? | +|-------|-----------|---------------|-------------| +| /code-review | required | Po implementacji, przed UNIFY | ○ | +| sonar-scanner | required | Po APPLY, przed UNIFY | ○ | + +**BLOKUJĄCE:** Wymagane skille MUSZĄ być wywołane przed UNIFY. + +## Checklist +- [ ] /code-review wywołany po implementacji +- [ ] sonar-scanner uruchomiony, wyniki sprawdzone i zaktualizowane w `DOCS/todo.md` + + + + + +## AC-1: `ShopproOrdersSyncService` nie zależy od `AllegroOrderSyncStateRepository` +```gherkin +Given plik `ShopproOrdersSyncService.php` +When sprawdzam import i konstruktor +Then nie ma żadnego odwołania do `AllegroOrderSyncStateRepository` — ani importu, ani typehinta +``` + +## AC-2: `ShopproOrdersSyncService` korzysta z `ShopproOrderSyncStateRepository` +```gherkin +Given plik `ShopproOrdersSyncService.php` +When sprawdzam konstruktor i metody wywołujące state +Then zmienna `$syncState` ma typ `ShopproOrderSyncStateRepository` i jest poprawnie wstrzyknięta +``` + +## AC-3: `ShopproOrderSyncStateRepository` ma identyczny kontrakt publiczny co `AllegroOrderSyncStateRepository` +```gherkin +Given nowy plik `ShopproOrderSyncStateRepository.php` +When sprawdzam publiczne metody +Then istnieją: `getState(int $integrationId)`, `markRunStarted(int, DateTimeImmutable)`, `markRunSuccess(int, DateTimeImmutable, ?string, ?string)`, `markRunFailed(int, DateTimeImmutable, string)` +``` + +## AC-4: `Application.php` wstrzykuje `ShopproOrderSyncStateRepository` +```gherkin +Given plik `Application.php`, linia tworzenia `ShopproOrdersSyncService` +When sprawdzam przekazywane zależności +Then zamiast `new AllegroOrderSyncStateRepository($this->db)` jest `new ShopproOrderSyncStateRepository($this->db)` +``` + +## AC-5: Brak regresji — zachowanie synchronizacji shopPRO nie zmienia się +```gherkin +Given działający mechanizm synchronizacji shopPRO (tabela `integration_order_sync_state`, logika kursorowa) +When uruchamiam cron lub ręczną synchronizację shopPRO +Then dane stanu są nadal poprawnie odczytywane i zapisywane do tej samej tabeli z tym samym `integration_id` +``` + + + + + + + Zadanie 1: Utwórz `ShopproOrderSyncStateRepository` + src/Modules/Settings/ShopproOrderSyncStateRepository.php + + Utwórz nowy plik `src/Modules/Settings/ShopproOrderSyncStateRepository.php`. + + Klasa ma być funkcjonalnie identyczna z `AllegroOrderSyncStateRepository` — ta sama tabela `integration_order_sync_state`, ta sama logika `resolveColumns()`, ten sam `upsertState()`. Skopiuj implementację i zmień tylko: + - Nazwę klasy: `ShopproOrderSyncStateRepository` (zamiast `AllegroOrderSyncStateRepository`) + - Namespace pozostaje `App\Modules\Settings` + - Deklaracja `final class ShopproOrderSyncStateRepository` + + Uwagi: + - Zachowaj `final` — klasa nie jest przewidziana do dziedziczenia + - Zachowaj identyczny kontrakt publiczny (te same sygnatury metod, te same typy zwracane) + - Nie zmieniaj nazw kolumn w tabeli ani logiki `resolveColumns()` — tabela jest wspólna, ale dane są scopowane przez `integration_id` + - Nie dodawaj żadnej nowej logiki — to tylko rename bez zmiany zachowania + + + Plik `src/Modules/Settings/ShopproOrderSyncStateRepository.php` istnieje i zawiera `final class ShopproOrderSyncStateRepository`. + `php -l src/Modules/Settings/ShopproOrderSyncStateRepository.php` — brak błędów składni. + Publiczne metody: `getState`, `markRunStarted`, `markRunSuccess`, `markRunFailed` — wszystkie obecne. + + AC-3 spełnione: nowe repozytorium z poprawnym kontraktem istnieje + + + + Zadanie 2: Zaktualizuj `ShopproOrdersSyncService` — zamień zależność + src/Modules/Settings/ShopproOrdersSyncService.php + + W `ShopproOrdersSyncService.php`: + + 1. Usuń import (use): `use App\Modules\Settings\AllegroOrderSyncStateRepository;` + (jeśli istnieje jako osobna linia — plik jest w tym samym namespace więc może go nie mieć) + + 2. Zmień typ parametru w konstruktorze: + ```php + // PRZED: + private readonly AllegroOrderSyncStateRepository $syncState, + // PO: + private readonly ShopproOrderSyncStateRepository $syncState, + ``` + + 3. Nie zmieniaj nazwy zmiennej `$syncState` — żaden inny kod w pliku nie wymaga zmian. + + Uwagi: + - Nie zmieniaj żadnych wywołań metod na `$this->syncState` — kontrakt jest identyczny + - Nie zmieniaj kolejności parametrów konstruktora + - Nie modyfikuj żadnej innej logiki w pliku + + + `grep -n "AllegroOrderSyncStateRepository" src/Modules/Settings/ShopproOrdersSyncService.php` — brak wyników. + `grep -n "ShopproOrderSyncStateRepository" src/Modules/Settings/ShopproOrdersSyncService.php` — obecny w typehintie konstruktora. + `php -l src/Modules/Settings/ShopproOrdersSyncService.php` — brak błędów składni. + + AC-1, AC-2 spełnione: serwis używa poprawnego repozytorium + + + + Zadanie 3: Zaktualizuj `Application.php` — wstrzyknij `ShopproOrderSyncStateRepository` + src/Core/Application.php + + W `Application.php`: + + 1. Dodaj import: `use App\Modules\Settings\ShopproOrderSyncStateRepository;` + (w bloku use, razem z pozostałymi importami z Settings) + + 2. Znajdź miejsce tworzenia `ShopproOrdersSyncService` (linia ~296–301): + ```php + $shopproSyncService = new ShopproOrdersSyncService( + ... + new AllegroOrderSyncStateRepository($this->db), + ... + ); + ``` + Zamień `new AllegroOrderSyncStateRepository($this->db)` na `new ShopproOrderSyncStateRepository($this->db)` — tylko w tym jednym miejscu (przy tworzeniu `ShopproOrdersSyncService`). + + 3. Sprawdź, czy `AllegroOrderSyncStateRepository` jest nadal używane gdzie indziej w `Application.php` (przy tworzeniu `AllegroOrderImportService` lub podobnych). Jeśli tak — nie usuwaj jego importu. Usuń import tylko jeśli nie ma innych użyć. + + Uwagi: + - Zachowaj kolejność argumentów konstruktora `ShopproOrdersSyncService` bez zmian + - Nie zmieniaj wierszy niezwiązanych z `ShopproOrdersSyncService` + + + W okolicach tworzenia `ShopproOrdersSyncService`: `new ShopproOrderSyncStateRepository($this->db)` jest obecne. + Import `use App\Modules\Settings\ShopproOrderSyncStateRepository;` istnieje. + `php -l src/Core/Application.php` — brak błędów składni. + + AC-4 spełnione: Application.php tworzy poprawne repozytorium dla shopPRO + + + + + + +## NIE ZMIENIAJ +- `src/Modules/Settings/AllegroOrderSyncStateRepository.php` — nie modyfikuj oryginalnej klasy Allegro (inne serwisy mogą jej używać) +- Tabela bazy danych `integration_order_sync_state` — nie dodawaj nowych kolumn, nie zmieniaj schematu +- Logiki biznesowej synchronizacji w `ShopproOrdersSyncService` — wyłącznie zmiana zależności +- Żadnych innych plików poza listą `files_modified` + +## LIMITY ZAKRESU +- Nie refaktoryzuj `ShopproOrdersSyncService` ponad wymaganą zmianę zależności (jest na to osobny concern — "1192 lines god class") +- Nie wydzielaj interfejsu `OrderSyncStateRepositoryInterface` — to osobna praca poza zakresem planu +- Nie przenoś klasy do innego namespace ani modułu + + + + +Przed deklaracją zakończenia planu: +- [ ] Plik `ShopproOrderSyncStateRepository.php` istnieje z poprawnymi metodami publicznymi +- [ ] `ShopproOrdersSyncService.php` nie zawiera żadnego odwołania do `AllegroOrderSyncStateRepository` +- [ ] `Application.php` wstrzykuje `ShopproOrderSyncStateRepository` przy tworzeniu `ShopproOrdersSyncService` +- [ ] `php -l` wszystkich 3 zmienionych plików PHP — brak błędów składni + + + +- Wszystkie 3 zadania ukończone +- Wszystkie 5 kryteriów akceptacji spełnione +- Brak błędów składni PHP +- Wpis z CONCERNS.md (Tech Debt — ShopproOrdersSyncService Uses AllegroOrderSyncStateRepository) usunięty po naprawieniu + + + +Po zakończeniu utwórz `.paul/phases/02-bug-fixes/02-03-SUMMARY.md` + diff --git a/.paul/phases/02-bug-fixes/02-03-SUMMARY.md b/.paul/phases/02-bug-fixes/02-03-SUMMARY.md new file mode 100644 index 0000000..0d04c76 --- /dev/null +++ b/.paul/phases/02-bug-fixes/02-03-SUMMARY.md @@ -0,0 +1,114 @@ +--- +phase: 02-bug-fixes +plan: 03 +subsystem: settings, dependency-injection +tags: [shoppro, repository, dependency, naming] + +requires: + - phase: 02-bug-fixes + provides: Plan 02-02 (last_status_checked_at cursor) + +provides: + - ShopproOrderSyncStateRepository (dedykowane repozytorium stanu dla shopPRO) + - ShopproOrdersSyncService wstrzykuje poprawną zależność + +affects: shoppro-sync, application-wiring + +tech-stack: + added: [] + patterns: [dedicated-repository-per-integration] + +key-files: + created: + - src/Modules/Settings/ShopproOrderSyncStateRepository.php + modified: + - src/Modules/Settings/ShopproOrdersSyncService.php + - src/Core/Application.php + +key-decisions: + - "Brak wspólnej klasy bazowej — ekstrakcja AbstractOrderSyncStateRepository poza zakresem planu" + - "Ta sama tabela integration_order_sync_state — zmiana tylko nazwy klasy, nie schematu" + +patterns-established: + - "Każda integracja ma własne dedykowane repozytorium stanu (AllegroOrderSyncStateRepository, ShopproOrderSyncStateRepository)" + +duration: ~20min +started: 2026-03-13T00:00:00Z +completed: 2026-03-13T00:00:00Z +--- + +# Phase 02 Plan 03: ShopproOrderSyncStateRepository + +**Wydzielono `ShopproOrderSyncStateRepository` — `ShopproOrdersSyncService` nie zależy już od klasy Allegro.** + +## Performance + +| Metric | Value | +|--------|-------| +| Duration | ~20min | +| Started | 2026-03-13 | +| Completed | 2026-03-13 | +| Tasks | 3/3 completed | +| Files modified | 3 (1 new, 2 modified) | + +## Acceptance Criteria Results + +| Criterion | Status | Notes | +|-----------|--------|-------| +| AC-1: ShopproOrdersSyncService nie zależy od AllegroOrderSyncStateRepository | Pass | 0 wystąpień w pliku | +| AC-2: ShopproOrdersSyncService używa ShopproOrderSyncStateRepository | Pass | Konstruktor linia 16 | +| AC-3: ShopproOrderSyncStateRepository ma identyczny kontrakt publiczny | Pass | getState, markRunStarted, markRunSuccess, markRunFailed | +| AC-4: Application.php wstrzykuje ShopproOrderSyncStateRepository | Pass | Linia 302 | +| AC-5: Brak regresji — ta sama tabela, ta sama logika | Pass | Kod identyczny, tylko nazwa klasy | + +## Accomplishments + +- Nowy `ShopproOrderSyncStateRepository` — identyczny kontrakt publiczny z `AllegroOrderSyncStateRepository` +- Poprawna zależność w `ShopproOrdersSyncService` (zmiana jednego typehinta w konstruktorze) +- `Application.php` wstrzykuje właściwe repozytorium do właściwego serwisu +- `AllegroOrderSyncStateRepository` zachowany nienaruszony — nadal używany przez `AllegroOrdersSyncService` + +## Files Created/Modified + +| File | Change | Purpose | +|------|--------|---------| +| `src/Modules/Settings/ShopproOrderSyncStateRepository.php` | Created | Dedykowane repozytorium stanu dla shopPRO | +| `src/Modules/Settings/ShopproOrdersSyncService.php` | Modified | Zamiana typehinta w konstruktorze | +| `src/Core/Application.php` | Modified | Nowy import + zamiana wstrzyknięcia | + +## Decisions Made + +| Decision | Rationale | Impact | +|----------|-----------|--------| +| Pełna kopia kodu zamiast abstrakcji | Ekstrakcja interfejsu/klasy bazowej poza zakresem planu (granica BOUNDARIES) | Tech debt: przyszła zmiana logiki = dwa miejsca do edycji | +| Ta sama tabela DB | `integration_id`-scoping zapewnia izolację bez nowego schematu | Zero risk migracji, brak destrukcji danych | + +## Deviations from Plan + +None — plan wykonany dokładnie jak napisano. + +## Issues Encountered + +None. + +## Skill Audit + +| Skill | Status | Notes | +|-------|--------|-------| +| /code-review | ✓ | Wywołany łącznie z planem 02-02 | +| sonar-scanner | ✓ | Uruchomiony — brak nowych issues | + +## Next Phase Readiness + +**Ready:** +- shopPRO sync używa poprawnie nazwanej zależności +- Wzorzec: każda integracja → własne dedykowane repozytorium stanu + +**Concerns:** +- `ShopproOrderSyncStateRepository` i `AllegroOrderSyncStateRepository` to identyczny kod — ryzyko rozbieżności przy przyszłych poprawkach. Do zaadresowania w osobnym planie (ekstrakcja wspólnej klasy bazowej). + +**Blockers:** None + +--- +*Phase: 02-bug-fixes, Plan: 03* +*Completed: 2026-03-13* diff --git a/DOCS/todo.md b/DOCS/todo.md index e73e47b..3f29041 100644 --- a/DOCS/todo.md +++ b/DOCS/todo.md @@ -15,3 +15,22 @@ 15. [] W tym miejscu odwróć kolejność: najpierw źródło potem ID,
f6079660-1af8-11f1-a7c9-231cf6ef29d1allegro
16. [] Na liście zamówień statusy powinno być pokolorowane zgodnie z ustawieniami. 17. [] Na liście zamówien jak jest źródło i id zamówienia to zamiast shopPRO musi pisać która integracja konkretnie. Oraz dodajemy napis ID: ...D + +## SonarQube — post plany 02-02 i 02-03 (skan 2026-03-13) +30. [] [Sonar 2026-03-13] Brak nowych issues — AllegroStatusSyncService i ShopproOrderSyncStateRepository czyste. Pre-existing issues w ShopproOrdersSyncService (god class) i Application.php niezmienione przez nasze modyfikacje. + +## SonarQube — post plan 01-01 (skan 2026-03-12) +28. [] [Sonar 2026-03-12] php:S1142 — AllegroTokenManager::resolveToken() ma 4 returny (powyżej limitu 3) (1x nowe) +29. [] [Sonar 2026-03-12] php:S112 — AllegroTokenManager rzuca generic RuntimeException zamiast dedykowanej klasy wyjątku (3x nowe) + +## SonarQube — code quality (327 issues, skan 2026-03-12) +18. [] php:S112 (95x) — zastąpić generic `new \Exception` konkretnymi klasami wyjątków +19. [] php:S1142 (57x) — zredukować liczbę `return` w metodach (early return → wydzielić metody) +20. [] php:S1192 (40x) — wyciągnąć powtarzające się string literals do stałych +21. [] php:S3776 (31x) — obniżyć złożoność kognitywną metod (wydzielić logikę do pomocniczych metod) +22. [] Web:S6827 (15x) — dodać brakujące atrybuty `alt` na tagach `` +23. [] Web:S6819 (12x) — poprawić dostępność HTML (accessibility) +24. [] php:S1172 (11x) — usunąć nieużywane parametry funkcji +25. [] php:S3358 (11x) — rozwinąć zagnieżdżone operatory ternarne +26. [] php:S1448 (6x) — podzielić klasy z za dużą liczbą metod +27. [] php:S138 (4x) — skrócić zbyt długie metody diff --git a/database/migrations/20260312_000047_add_last_status_checked_at_to_orders.sql b/database/migrations/20260312_000047_add_last_status_checked_at_to_orders.sql new file mode 100644 index 0000000..632bbab --- /dev/null +++ b/database/migrations/20260312_000047_add_last_status_checked_at_to_orders.sql @@ -0,0 +1,8 @@ +ALTER TABLE orders + ADD COLUMN last_status_checked_at DATETIME NULL DEFAULT NULL + COMMENT 'Timestamp ostatniego sprawdzenia statusu przez AllegroStatusSyncService' + AFTER source_updated_at; + +-- Indeks kompozytowy wspiera zapytanie filtrujące po source + sortujące po source_updated_at +ALTER TABLE orders + ADD INDEX orders_source_updated_idx (source, source_updated_at); diff --git a/src/Core/Application.php b/src/Core/Application.php index f120f90..c885e8b 100644 --- a/src/Core/Application.php +++ b/src/Core/Application.php @@ -30,11 +30,13 @@ use App\Modules\Settings\AllegroOrdersSyncService; use App\Modules\Settings\AllegroOrderSyncStateRepository; use App\Modules\Settings\AllegroOAuthClient; use App\Modules\Settings\AllegroStatusSyncService; +use App\Modules\Settings\AllegroTokenManager; use App\Modules\Settings\AllegroStatusMappingRepository; use App\Modules\Settings\OrderStatusRepository; use App\Modules\Settings\ShopproApiClient; use App\Modules\Settings\ShopproIntegrationsRepository; use App\Modules\Settings\ShopproOrdersSyncService; +use App\Modules\Settings\ShopproOrderSyncStateRepository; use App\Modules\Settings\ShopproPaymentStatusSyncService; use App\Modules\Settings\ShopproStatusSyncService; use App\Modules\Settings\ShopproStatusMappingRepository; @@ -274,11 +276,12 @@ final class Application (string) $this->config('app.integrations.secret', '') ); $oauthClient = new AllegroOAuthClient(); + $tokenManager = new AllegroTokenManager($integrationRepository, $oauthClient); $apiClient = new AllegroApiClient(); $statusMappingRepository = new AllegroStatusMappingRepository($this->db); $orderImportService = new AllegroOrderImportService( $integrationRepository, - $oauthClient, + $tokenManager, $apiClient, new OrderImportRepository($this->db), $statusMappingRepository, @@ -287,7 +290,7 @@ final class Application $ordersSyncService = new AllegroOrdersSyncService( $integrationRepository, new AllegroOrderSyncStateRepository($this->db), - $oauthClient, + $tokenManager, $apiClient, $orderImportService ); @@ -296,7 +299,7 @@ final class Application $this->db, (string) $this->config('app.integrations.secret', '') ), - new AllegroOrderSyncStateRepository($this->db), + new ShopproOrderSyncStateRepository($this->db), new ShopproApiClient(), new OrderImportRepository($this->db), new ShopproStatusMappingRepository($this->db), diff --git a/src/Modules/Settings/AllegroStatusSyncService.php b/src/Modules/Settings/AllegroStatusSyncService.php index cdf1e7d..729e107 100644 --- a/src/Modules/Settings/AllegroStatusSyncService.php +++ b/src/Modules/Settings/AllegroStatusSyncService.php @@ -60,6 +60,7 @@ final class AllegroStatusSyncService try { $this->orderImportService->importSingleOrder($sourceOrderId); $result['processed']++; + $this->markOrderStatusChecked((int) ($order['id'] ?? 0)); } catch (Throwable $exception) { $result['failed']++; $errors = is_array($result['errors']) ? $result['errors'] : []; @@ -89,6 +90,7 @@ final class AllegroStatusSyncService FROM orders WHERE source = ? AND LOWER(COALESCE(external_status_id, "")) NOT IN (' . $placeholders . ') + AND (last_status_checked_at IS NULL OR source_updated_at > last_status_checked_at) ORDER BY source_updated_at DESC LIMIT ' . self::MAX_ORDERS_PER_RUN ); @@ -100,4 +102,14 @@ final class AllegroStatusSyncService return []; } } + + private function markOrderStatusChecked(int $orderId): void + { + try { + $statement = $this->pdo->prepare('UPDATE orders SET last_status_checked_at = NOW() WHERE id = ?'); + $statement->execute([$orderId]); + } catch (Throwable) { + // Błąd zapisu logu nie powinien przerywać pętli synchronizacji + } + } } diff --git a/src/Modules/Settings/ShopproOrderSyncStateRepository.php b/src/Modules/Settings/ShopproOrderSyncStateRepository.php new file mode 100644 index 0000000..28f21bf --- /dev/null +++ b/src/Modules/Settings/ShopproOrderSyncStateRepository.php @@ -0,0 +1,270 @@ +defaultState(); + if ($integrationId <= 0) { + return $default; + } + + $columns = $this->resolveColumns(); + if (!$columns['has_table']) { + return $default; + } + + $updatedAtColumn = $columns['updated_at_column']; + $sourceOrderIdColumn = $columns['source_order_id_column']; + if ($updatedAtColumn === null || $sourceOrderIdColumn === null) { + return $default; + } + + $selectParts = [ + $updatedAtColumn . ' AS last_synced_updated_at', + $sourceOrderIdColumn . ' AS last_synced_source_order_id', + 'last_run_at', + $columns['has_last_success_at'] ? 'last_success_at' : 'NULL AS last_success_at', + 'last_error', + ]; + + try { + $statement = $this->pdo->prepare( + 'SELECT ' . implode(', ', $selectParts) . ' + FROM integration_order_sync_state + WHERE integration_id = :integration_id + LIMIT 1' + ); + $statement->execute(['integration_id' => $integrationId]); + $row = $statement->fetch(PDO::FETCH_ASSOC); + } catch (Throwable) { + return $default; + } + + if (!is_array($row)) { + return $default; + } + + return [ + 'last_synced_updated_at' => StringHelper::nullableString((string) ($row['last_synced_updated_at'] ?? '')), + 'last_synced_source_order_id' => StringHelper::nullableString((string) ($row['last_synced_source_order_id'] ?? '')), + 'last_run_at' => StringHelper::nullableString((string) ($row['last_run_at'] ?? '')), + 'last_success_at' => StringHelper::nullableString((string) ($row['last_success_at'] ?? '')), + 'last_error' => StringHelper::nullableString((string) ($row['last_error'] ?? '')), + ]; + } + + public function markRunStarted(int $integrationId, DateTimeImmutable $now): void + { + $this->upsertState($integrationId, [ + 'last_run_at' => $now->format('Y-m-d H:i:s'), + ]); + } + + public function markRunFailed(int $integrationId, DateTimeImmutable $now, string $error): void + { + $this->upsertState($integrationId, [ + 'last_run_at' => $now->format('Y-m-d H:i:s'), + 'last_error' => mb_substr(trim($error), 0, 500), + ]); + } + + public function markRunSuccess( + int $integrationId, + DateTimeImmutable $now, + ?string $lastSyncedUpdatedAt, + ?string $lastSyncedSourceOrderId + ): void { + $changes = [ + 'last_run_at' => $now->format('Y-m-d H:i:s'), + 'last_error' => null, + ]; + + if ($lastSyncedUpdatedAt !== null) { + $changes['last_synced_updated_at'] = $lastSyncedUpdatedAt; + } + if ($lastSyncedSourceOrderId !== null) { + $changes['last_synced_source_order_id'] = $lastSyncedSourceOrderId; + } + + $this->upsertState($integrationId, $changes, true); + } + + /** + * @param array $changes + */ + private function upsertState(int $integrationId, array $changes, bool $setSuccessAt = false): void + { + if ($integrationId <= 0) { + return; + } + + $columns = $this->resolveColumns(); + if (!$columns['has_table']) { + return; + } + + $updatedAtColumn = $columns['updated_at_column']; + $sourceOrderIdColumn = $columns['source_order_id_column']; + if ($updatedAtColumn === null || $sourceOrderIdColumn === null) { + return; + } + + $insertColumns = ['integration_id', 'created_at', 'updated_at']; + $insertValues = [':integration_id', ':created_at', ':updated_at']; + $updateParts = ['updated_at = VALUES(updated_at)']; + $params = [ + 'integration_id' => $integrationId, + 'created_at' => date('Y-m-d H:i:s'), + 'updated_at' => date('Y-m-d H:i:s'), + ]; + + $columnMap = [ + 'last_run_at' => 'last_run_at', + 'last_error' => 'last_error', + 'last_synced_updated_at' => $updatedAtColumn, + 'last_synced_source_order_id' => $sourceOrderIdColumn, + ]; + + foreach ($columnMap as $inputKey => $columnName) { + if (!array_key_exists($inputKey, $changes)) { + continue; + } + + $paramName = $inputKey; + $insertColumns[] = $columnName; + $insertValues[] = ':' . $paramName; + $updateParts[] = $columnName . ' = VALUES(' . $columnName . ')'; + $params[$paramName] = $changes[$inputKey]; + } + + if ($setSuccessAt && $columns['has_last_success_at']) { + $insertColumns[] = 'last_success_at'; + $insertValues[] = ':last_success_at'; + $updateParts[] = 'last_success_at = VALUES(last_success_at)'; + $params['last_success_at'] = date('Y-m-d H:i:s'); + } + + try { + $statement = $this->pdo->prepare( + 'INSERT INTO integration_order_sync_state (' . implode(', ', $insertColumns) . ') + VALUES (' . implode(', ', $insertValues) . ') + ON DUPLICATE KEY UPDATE ' . implode(', ', $updateParts) + ); + $statement->execute($params); + } catch (Throwable) { + return; + } + } + + /** + * @return array{ + * has_table:bool, + * updated_at_column:?string, + * source_order_id_column:?string, + * has_last_success_at:bool + * } + */ + private function resolveColumns(): array + { + if ($this->columns !== null) { + return $this->columns; + } + + $result = [ + 'has_table' => false, + 'updated_at_column' => null, + 'source_order_id_column' => null, + 'has_last_success_at' => false, + ]; + + try { + $statement = $this->pdo->prepare( + 'SELECT COLUMN_NAME + FROM information_schema.COLUMNS + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME = "integration_order_sync_state"' + ); + $statement->execute(); + $rows = $statement->fetchAll(PDO::FETCH_COLUMN); + } catch (Throwable) { + $this->columns = $result; + return $result; + } + + if (!is_array($rows) || $rows === []) { + $this->columns = $result; + return $result; + } + + $available = []; + foreach ($rows as $columnName) { + $name = trim((string) $columnName); + if ($name === '') { + continue; + } + $available[$name] = true; + } + + $result['has_table'] = true; + if (isset($available['last_synced_order_updated_at'])) { + $result['updated_at_column'] = 'last_synced_order_updated_at'; + } elseif (isset($available['last_synced_external_updated_at'])) { + $result['updated_at_column'] = 'last_synced_external_updated_at'; + } + + if (isset($available['last_synced_source_order_id'])) { + $result['source_order_id_column'] = 'last_synced_source_order_id'; + } elseif (isset($available['last_synced_external_order_id'])) { + $result['source_order_id_column'] = 'last_synced_external_order_id'; + } + + $result['has_last_success_at'] = isset($available['last_success_at']); + $this->columns = $result; + + return $result; + } + + /** + * @return array{ + * last_synced_updated_at:?string, + * last_synced_source_order_id:?string, + * last_run_at:?string, + * last_success_at:?string, + * last_error:?string + * } + */ + private function defaultState(): array + { + return [ + 'last_synced_updated_at' => null, + 'last_synced_source_order_id' => null, + 'last_run_at' => null, + 'last_success_at' => null, + 'last_error' => null, + ]; + } +} diff --git a/src/Modules/Settings/ShopproOrdersSyncService.php b/src/Modules/Settings/ShopproOrdersSyncService.php index da62a75..d36a004 100644 --- a/src/Modules/Settings/ShopproOrdersSyncService.php +++ b/src/Modules/Settings/ShopproOrdersSyncService.php @@ -13,7 +13,7 @@ final class ShopproOrdersSyncService { public function __construct( private readonly ShopproIntegrationsRepository $integrations, - private readonly AllegroOrderSyncStateRepository $syncState, + private readonly ShopproOrderSyncStateRepository $syncState, private readonly ShopproApiClient $apiClient, private readonly OrderImportRepository $orderImportRepository, private readonly ShopproStatusMappingRepository $statusMappings, diff --git a/src/Modules/Shipments/AllegroShipmentService.php b/src/Modules/Shipments/AllegroShipmentService.php index 0aae25a..ce6eaed 100644 --- a/src/Modules/Shipments/AllegroShipmentService.php +++ b/src/Modules/Shipments/AllegroShipmentService.php @@ -5,19 +5,15 @@ namespace App\Modules\Shipments; use App\Modules\Orders\OrdersRepository; use App\Modules\Settings\AllegroApiClient; -use App\Modules\Settings\AllegroIntegrationRepository; -use App\Modules\Settings\AllegroOAuthClient; +use App\Modules\Settings\AllegroTokenManager; use App\Modules\Settings\CompanySettingsRepository; -use DateInterval; -use DateTimeImmutable; use RuntimeException; use Throwable; final class AllegroShipmentService implements ShipmentProviderInterface { public function __construct( - private readonly AllegroIntegrationRepository $integrationRepository, - private readonly AllegroOAuthClient $oauthClient, + private readonly AllegroTokenManager $tokenManager, private readonly AllegroApiClient $apiClient, private readonly ShipmentPackageRepository $packages, private readonly CompanySettingsRepository $companySettings, @@ -35,7 +31,7 @@ final class AllegroShipmentService implements ShipmentProviderInterface */ public function getDeliveryServices(): array { - [$accessToken, $env] = $this->resolveToken(); + [$accessToken, $env] = $this->tokenManager->resolveToken(); $response = $this->apiClient->getDeliveryServices($env, $accessToken); return is_array($response['services'] ?? null) ? $response['services'] : []; } @@ -145,13 +141,13 @@ final class AllegroShipmentService implements ShipmentProviderInterface 'payload_json' => $apiPayload, ]); - [$accessToken, $env] = $this->resolveToken(); + [$accessToken, $env] = $this->tokenManager->resolveToken(); try { $response = $this->apiClient->createShipment($env, $accessToken, $apiPayload); } catch (RuntimeException $exception) { if (trim($exception->getMessage()) === 'ALLEGRO_HTTP_401') { - [$accessToken, $env] = $this->forceRefreshToken(); + [$accessToken, $env] = $this->tokenManager->resolveToken(); $response = $this->apiClient->createShipment($env, $accessToken, $apiPayload); } else { $this->packages->update($packageId, [ @@ -189,7 +185,7 @@ final class AllegroShipmentService implements ShipmentProviderInterface throw new RuntimeException('Brak command_id dla tej paczki.'); } - [$accessToken, $env] = $this->resolveToken(); + [$accessToken, $env] = $this->tokenManager->resolveToken(); $response = $this->apiClient->getShipmentCreationStatus($env, $accessToken, $commandId); $status = strtoupper(trim((string) ($response['status'] ?? ''))); @@ -250,9 +246,9 @@ final class AllegroShipmentService implements ShipmentProviderInterface throw new RuntimeException('Przesylka nie zostala jeszcze utworzona.'); } - [$accessToken, $env] = $this->resolveToken(); + [$accessToken, $env] = $this->tokenManager->resolveToken(); $labelFormat = trim((string) ($package['label_format'] ?? 'PDF')); - $pageSize = $labelFormat === 'ZPL' ? 'A6' : 'A6'; + $pageSize = $labelFormat === 'ZPL' ? 'A6' : 'A4'; $binary = $this->apiClient->getShipmentLabel($env, $accessToken, [$shipmentId], $pageSize); $dir = rtrim($storagePath, '/\\') . '/labels'; @@ -361,85 +357,6 @@ final class AllegroShipmentService implements ShipmentProviderInterface } } - /** - * @return array{0: string, 1: string} - */ - private function resolveToken(): array - { - $oauth = $this->integrationRepository->getTokenCredentials(); - if ($oauth === null) { - throw new RuntimeException('Brak polaczenia OAuth Allegro. Polacz konto w Ustawieniach.'); - } - - $env = (string) ($oauth['environment'] ?? 'sandbox'); - $accessToken = trim((string) ($oauth['access_token'] ?? '')); - $tokenExpiresAt = trim((string) ($oauth['token_expires_at'] ?? '')); - - if ($accessToken === '') { - return $this->forceRefreshToken(); - } - - if ($tokenExpiresAt !== '') { - try { - $expiresAt = new DateTimeImmutable($tokenExpiresAt); - if ($expiresAt <= (new DateTimeImmutable('now'))->add(new DateInterval('PT5M'))) { - return $this->forceRefreshToken(); - } - } catch (Throwable) { - return $this->forceRefreshToken(); - } - } - - return [$accessToken, $env]; - } - - /** - * @return array{0: string, 1: string} - */ - private function forceRefreshToken(): array - { - $oauth = $this->integrationRepository->getTokenCredentials(); - if ($oauth === null) { - throw new RuntimeException('Brak danych OAuth Allegro.'); - } - - $token = $this->oauthClient->refreshAccessToken( - (string) ($oauth['environment'] ?? 'sandbox'), - (string) ($oauth['client_id'] ?? ''), - (string) ($oauth['client_secret'] ?? ''), - (string) ($oauth['refresh_token'] ?? '') - ); - - $expiresAt = null; - $expiresIn = max(0, (int) ($token['expires_in'] ?? 0)); - if ($expiresIn > 0) { - $expiresAt = (new DateTimeImmutable('now')) - ->add(new DateInterval('PT' . $expiresIn . 'S')) - ->format('Y-m-d H:i:s'); - } - - $refreshToken = trim((string) ($token['refresh_token'] ?? '')); - if ($refreshToken === '') { - $refreshToken = (string) ($oauth['refresh_token'] ?? ''); - } - - $this->integrationRepository->saveTokens( - (string) ($token['access_token'] ?? ''), - $refreshToken, - (string) ($token['token_type'] ?? ''), - (string) ($token['scope'] ?? ''), - $expiresAt - ); - - $updated = $this->integrationRepository->getTokenCredentials(); - $newToken = trim((string) ($updated['access_token'] ?? '')); - if ($newToken === '') { - throw new RuntimeException('Nie udalo sie odswiezyc tokenu Allegro.'); - } - - return [$newToken, (string) ($updated['environment'] ?? 'sandbox')]; - } - private function generateUuid(): string { $data = random_bytes(16);