fix(02-bug-fixes): fix 3 known bugs from CONCERNS.md
Phase 02 plans 02-01, 02-02, 02-03: - fix(02-01): dead condition in AllegroShipmentService ZPL page size Both ternary branches returned 'A6'; ZPL now correctly returns 'ZPL' - fix(02-02): add last_status_checked_at cursor to AllegroStatusSyncService New migration adds orders.last_status_checked_at DATETIME NULL with composite index (source, source_updated_at). findOrdersNeedingStatusSync() filters by cursor; markOrderStatusChecked() records timestamp on success. - fix(02-03): replace AllegroOrderSyncStateRepository in ShopproOrdersSyncService New ShopproOrderSyncStateRepository (same table, correct class name). Application.php wires correct repository to correct service. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
192
.paul/phases/02-bug-fixes/02-02-PLAN.md
Normal file
192
.paul/phases/02-bug-fixes/02-02-PLAN.md
Normal file
@@ -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
|
||||
---
|
||||
|
||||
<objective>
|
||||
## 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
|
||||
</objective>
|
||||
|
||||
<context>
|
||||
## 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
|
||||
</context>
|
||||
|
||||
<skills>
|
||||
## 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`
|
||||
|
||||
</skills>
|
||||
|
||||
<acceptance_criteria>
|
||||
|
||||
## 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)
|
||||
```
|
||||
|
||||
</acceptance_criteria>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto">
|
||||
<name>Zadanie 1: Migracja — dodaj kolumnę `last_status_checked_at` do `orders`</name>
|
||||
<files>database/migrations/20260312_000047_add_last_status_checked_at_to_orders.sql</files>
|
||||
<action>
|
||||
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
|
||||
</action>
|
||||
<verify>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`</verify>
|
||||
<done>AC-4 spełnione: migracja gotowa do uruchomienia</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Zadanie 2: Zaktualizuj `AllegroStatusSyncService` — kursor `last_status_checked_at`</name>
|
||||
<files>src/Modules/Settings/AllegroStatusSyncService.php</files>
|
||||
<action>
|
||||
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
|
||||
</action>
|
||||
<verify>
|
||||
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()`
|
||||
</verify>
|
||||
<done>AC-1, AC-2, AC-3 spełnione: kursor działa, timestamp zapisywany po sukcesie</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<boundaries>
|
||||
|
||||
## 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
|
||||
|
||||
</boundaries>
|
||||
|
||||
<verification>
|
||||
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
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- 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
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Po zakończeniu utwórz `.paul/phases/02-bug-fixes/02-02-SUMMARY.md`
|
||||
</output>
|
||||
Reference in New Issue
Block a user