shopPRO/.paul/phases/05-order-bugs-fix/05-01-FIX.md

---
phase: 05-order-bugs-fix
plan: 05-01
type: fix
wave: 1
depends_on: []
files_modified:
  - autoload/front/Controllers/ShopBasketController.php
  - autoload/Domain/Order/OrderRepository.php
  - autoload/Domain/PaymentMethod/PaymentMethodRepository.php
  - autoload/admin/Controllers/ShopPaymentMethodController.php
  - migrations/0.338.sql
  - docs/DATABASE_STRUCTURE.md
autonomous: true
---

<objective>
## Goal
Fix 2 production bugs reported by customer: (1) duplicate orders on retry after error, (2) wrong initial status for cash-on-delivery orders.

## Purpose
Production issues affecting real customers. Bug 1 causes double-billed orders. Bug 2 causes wrong order flow for COD payments.

## Output
- `summaryView()` guards against re-submission after successful order
- `basketSave()` handles exceptions from `createFromBasket()` safely
- `is_cod` column added to `pp_shop_payment_methods`
- COD status promotion uses `is_cod` flag instead of hardcoded `payment_id == 3`
- Admin form for payment methods shows `is_cod` switch
</objective>

<context>
@.paul/STATE.md
@.paul/ROADMAP.md
@autoload/front/Controllers/ShopBasketController.php
@autoload/Domain/Order/OrderRepository.php
@autoload/Domain/PaymentMethod/PaymentMethodRepository.php
@autoload/admin/Controllers/ShopPaymentMethodController.php
</context>

<acceptance_criteria>
## AC-1: No duplicate order on retry
Given a customer submits an order and it is created successfully (order_id saved in session),
When the customer navigates back to `/podsumowanie` and tries to submit again,
Then they are redirected to the existing order page — no new order is created.

## AC-2: Exception in createFromBasket does not duplicate order
Given `createFromBasket()` throws an uncaught exception after the INSERT succeeds (partial failure),
When the customer retries submission with the same basket,
Then the exception is caught, an error message is shown, basket session is preserved, and no second order is inserted via normal retry flow (AC-1 guards subsequent summary visit).

## AC-3: COD flag is configurable in admin
Given an admin opens any payment method in `/admin/shop_payment_method/edit/`,
When they toggle "Płatność przy odbiorze" switch and save,
Then the `is_cod` flag is persisted in `pp_shop_payment_methods.is_cod`.

## AC-4: COD order gets correct initial status
Given a customer places an order with a payment method where `is_cod = 1`,
When the order is created,
Then `pp_shop_order_statuses` contains status_id = 4 ("Przyjęte do realizacji") and the old status 0 entry is updated.
</acceptance_criteria>

<tasks>

<task type="auto">
  <name>Fix BUG-1: Guard summaryView() against re-submission after successful order</name>
  <files>autoload/front/Controllers/ShopBasketController.php</files>
  <action>
In `summaryView()`, BEFORE calling `createOrderSubmitToken()`, check if `ORDER_SUBMIT_LAST_ORDER_ID_SESSION_KEY` is set in session. If it is, look up that order's hash via `$this->orderRepository->findHashById($existingOrderId)`. If the hash exists, redirect to `/zamowienie/{hash}` and exit.

This means the customer who navigates back to the summary page after a successful order is immediately redirected to their order instead of seeing the form again (which would regenerate a token and allow double-submission).

Do NOT call `createOrderSubmitToken()` in this guard path — just redirect.

Current problematic code at the top of `summaryView()`:
```php
$orderSubmitToken = $this->createOrderSubmitToken();
```
Must become:
```php
$existingOrderId = isset($_SESSION[self::ORDER_SUBMIT_LAST_ORDER_ID_SESSION_KEY])
    ? (int)$_SESSION[self::ORDER_SUBMIT_LAST_ORDER_ID_SESSION_KEY]
    : 0;
if ($existingOrderId > 0) {
    $existingOrderHash = $this->orderRepository->findHashById($existingOrderId);
    if ($existingOrderHash) {
        header('Location: /zamowienie/' . $existingOrderHash);
        exit;
    }
}
$orderSubmitToken = $this->createOrderSubmitToken();
```
  </action>
  <verify>
    1. Create a test order successfully
    2. Navigate back to /podsumowanie in the same browser session
    3. Confirm browser redirects to /zamowienie/{hash} without showing the summary form
  </verify>
  <done>AC-1 satisfied: navigating back to summary after successful order redirects, no form shown</done>
</task>

<task type="auto">
  <name>Fix BUG-1: Wrap createFromBasket in try-catch in basketSave()</name>
  <files>autoload/front/Controllers/ShopBasketController.php</files>
  <action>
In `basketSave()`, wrap the call to `$this->orderRepository->createFromBasket(...)` in a try-catch block. On exception: log with `error_log()`, show user error message via `Helpers::error()`, and redirect to `/koszyk`. Do NOT clear the basket session in the catch block.

Replace the current `if ($order_id = $this->orderRepository->createFromBasket(...))` pattern with:

```php
$order_id = null;
try {
    $order_id = $this->orderRepository->createFromBasket(
        // ... all current args unchanged ...
    );
} catch (\Exception $e) {
    error_log('[basketSave] createFromBasket exception: ' . $e->getMessage());
    \Shared\Helpers\Helpers::error(\Shared\Helpers\Helpers::lang('zamowienie-zostalo-zlozone-komunikat-blad'));
    header('Location: /koszyk');
    exit;
}

if ($order_id) {
    // ... existing success block unchanged ...
} else {
    // ... existing error block unchanged ...
}
```

Use `\Exception` catch (not `\Throwable`) — the project targets PHP 7.4 which supports both, but `\Exception` covers the common cases (DB exceptions, mail exceptions). If there are any `\Error` throws in the chain they won't be caught — acceptable tradeoff for PHP 7.4 compatibility.
  </action>
  <verify>
    Confirm no PHP syntax errors: `php -l autoload/front/Controllers/ShopBasketController.php`
  </verify>
  <done>AC-2 satisfied: exceptions from createFromBasket are caught and handled gracefully</done>
</task>

<task type="auto">
  <name>Fix BUG-2: Add is_cod column migration</name>
  <files>migrations/0.338.sql, docs/DATABASE_STRUCTURE.md</files>
  <action>
Create the migration file at `migrations/0.338.sql` (kolejna wersja po 0.337):

```sql
ALTER TABLE `pp_shop_payment_methods`
  ADD COLUMN `is_cod` TINYINT(1) NOT NULL DEFAULT 0
  COMMENT 'Platnosc przy odbiorze (cash on delivery): 1 = tak, 0 = nie';
```

Also update `docs/DATABASE_STRUCTURE.md` — in the `pp_shop_payment_methods` table section, add the new column:
| is_cod | Płatność przy odbiorze: 1 = tak, 0 = nie (TINYINT DEFAULT 0) |

The migration must be run on production DB manually (document this in the plan summary).
  </action>
  <verify>
    File `migrations/0.338.sql` exists and contains valid ALTER TABLE statement.
    `docs/DATABASE_STRUCTURE.md` mentions `is_cod` in `pp_shop_payment_methods` section.
  </verify>
  <done>AC-3 precondition: column definition prepared for migration</done>
</task>

<task type="auto">
  <name>Fix BUG-2: Add is_cod to PaymentMethodRepository normalization and queries</name>
  <files>autoload/Domain/PaymentMethod/PaymentMethodRepository.php</files>
  <action>
1. In `normalizePaymentMethod(array $row)`: add `$row['is_cod'] = (int)($row['is_cod'] ?? 0);`

2. In `findActiveById()`: the method already uses `SELECT *` via Medoo `get('pp_shop_payment_methods', '*', ...)` so `is_cod` will be included automatically once the column exists.

3. In `forTransport()`: the method uses explicit column list in raw SQL. Add `spm.is_cod` to the SELECT list (around line ~241, alongside `spm.apilo_payment_type_id`).

4. In `paymentMethodsByTransport()` (if exists as a separate raw SQL method): similarly add `spm.is_cod` to the SELECT. Search for any other raw SQL selects in this file that list columns explicitly and add `is_cod` to them.

5. In the `allActive()` / `paymentMethodsCached()` path: if `allActive()` uses raw SQL with explicit columns, add `spm.is_cod` there too. If it uses `SELECT *`, nothing needed.

Cache keys that include payment method data (`payment_method{id}`, `payment_methods`) will return stale data until Redis is flushed. The post-deploy step is to flush Redis cache.
  </action>
  <verify>
    `php -l autoload/Domain/PaymentMethod/PaymentMethodRepository.php` — no syntax errors.
    All explicit SQL SELECTs in this file now include `is_cod`.
  </verify>
  <done>AC-3 + AC-4 precondition: repository returns is_cod field</done>
</task>

<task type="auto">
  <name>Fix BUG-2: Add is_cod switch to admin payment method form</name>
  <files>autoload/admin/Controllers/ShopPaymentMethodController.php</files>
  <action>
In `buildFormViewModel()`:

1. Add `'is_cod' => (int)($paymentMethod['is_cod'] ?? 0)` to the `$data` array.

2. Add a switch field after the `status` field:
```php
FormField::switch('is_cod', [
    'label' => 'Platnosc przy odbiorze',
    'tab' => 'settings',
]),
```

In the `save()` / `update()` method of this controller: ensure `is_cod` is read from POST and included in the DB update data. Find where the other fields (description, status, apilo_payment_type_id, etc.) are read from request and add:
```php
'is_cod' => (int)(\Shared\Helpers\Helpers::get('is_cod') ? 1 : 0),
```

Check if there is a `FormRequestHandler` or similar save mechanism — if so, `is_cod` may need to be added to the allowed fields list. Read the save method to confirm.
  </action>
  <verify>
    `php -l autoload/admin/Controllers/ShopPaymentMethodController.php` — no syntax errors.
    Check that `is_cod` appears in both the form field list and the save data array.
  </verify>
  <done>AC-3 satisfied: admin can set is_cod flag on any payment method</done>
</task>

<task type="auto">
  <name>Fix BUG-2: Use is_cod flag instead of hardcoded payment_id == 3 in OrderRepository</name>
  <files>autoload/Domain/Order/OrderRepository.php</files>
  <action>
In `createFromBasket()`, at lines 817-820, replace the hardcoded check:

```php
// BEFORE:
if ($payment_id == 3) {
    $this->updateOrderStatus($order_id, 4);
    $this->insertStatusHistory($order_id, 4, 1);
}
```

With:
```php
// AFTER:
if (!empty($payment_method['is_cod'])) {
    $this->updateOrderStatus($order_id, 4);
    $this->insertStatusHistory($order_id, 4, 1);
}
```

`$payment_method` is already fetched at line 669:
```php
$payment_method = ( new \Domain\PaymentMethod\PaymentMethodRepository( $this->db ) )->findActiveById( (int)$payment_id );
```
So `$payment_method['is_cod']` is available without any additional DB query.
  </action>
  <verify>
    `php -l autoload/Domain/Order/OrderRepository.php` — no syntax errors.
    Confirm the old `$payment_id == 3` no longer exists in createFromBasket().
  </verify>
  <done>AC-4 satisfied: COD status promotion is driven by is_cod flag, not hardcoded ID</done>
</task>

<task type="checkpoint:human-action" gate="blocking">
  <action>Run the database migration on production server</action>
  <instructions>
    Claude has prepared the migration file at `migrations/0.338.sql`.
    The SQL is: ALTER TABLE pp_shop_payment_methods ADD COLUMN is_cod TINYINT(1) NOT NULL DEFAULT 0

    You need to run this on the production database manually (via phpMyAdmin, SSH, or your DB client).

    After running, go to /admin/shop_payment_method/list/ → edit the "Płatność przy odbiorze" payment method → enable the "Płatnosc przy odbiorze" switch → Save.

    Also flush Redis cache (or wait for TTL expiry — payment methods cache is 24h).
  </instructions>
  <verification>
    Claude will verify the code changes are in place. The DB migration must be confirmed by you.
  </verification>
  <resume-signal>Type "done" when migration and admin flag set</resume-signal>
</task>

</tasks>

<boundaries>
## DO NOT CHANGE
- The CSRF token mechanism (separate from order submit token)
- The basket session structure
- The order submission token logic (ORDER_SUBMIT_TOKEN_SESSION_KEY) — only guard summaryView, don't change how tokens are generated/consumed
- Email sending logic in createFromBasket
- Any other payment method fields or behavior

## SCOPE LIMITS
- Do NOT add database-level unique constraints or idempotency key columns to pp_shop_orders (over-engineering for now)
- Do NOT change the order status values or their meaning
- Do NOT modify test files unless directly testing the changed methods
- Do NOT change the frontend templates
</boundaries>

<verification>
Before declaring plan complete:
- [ ] `php -l` passes on all modified PHP files
- [ ] summaryView() guard redirects to existing order when ORDER_SUBMIT_LAST_ORDER_ID_SESSION_KEY is set
- [ ] createFromBasket call in basketSave() is wrapped in try-catch
- [ ] `is_cod` column exists in migration SQL
- [ ] normalizePaymentMethod() includes is_cod normalization
- [ ] admin form shows is_cod switch
- [ ] admin save includes is_cod in update data
- [ ] OrderRepository uses $payment_method['is_cod'] not $payment_id == 3
- [ ] DATABASE_STRUCTURE.md updated
</verification>

<success_criteria>
- All PHP files lint-clean
- No more duplicate orders when customer navigates back to summary after successful order
- COD payment method (when is_cod=1) automatically promotes order to status 4
- Admin can configure which payment method is COD
</success_criteria>

<output>
After completion, create `.paul/phases/05-order-bugs-fix/05-01-FIX-SUMMARY.md` with:
- List of files changed
- Note that DB migration in `migrations/0.338.sql` must be run on production
- Note that admin must set is_cod=1 on the COD payment method after migration

Then run: `/koniec-pracy`
</output>