Files
shopPRO/.paul/phases/05-order-bugs-fix/05-01-FIX.md
2026-03-12 13:36:06 +01:00

13 KiB

phase, plan, type, wave, depends_on, files_modified, autonomous
phase plan type wave depends_on files_modified autonomous
05-order-bugs-fix 05-01 fix 1
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
true
## 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
@.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

<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>

Fix BUG-1: Guard summaryView() against re-submission after successful order autoload/front/Controllers/ShopBasketController.php 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():

$orderSubmitToken = $this->createOrderSubmitToken();

Must become:

$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();
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 AC-1 satisfied: navigating back to summary after successful order redirects, no form shown Fix BUG-1: Wrap createFromBasket in try-catch in basketSave() autoload/front/Controllers/ShopBasketController.php 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:

$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. Confirm no PHP syntax errors: php -l autoload/front/Controllers/ShopBasketController.php AC-2 satisfied: exceptions from createFromBasket are caught and handled gracefully

Fix BUG-2: Add is_cod column migration migrations/0.338.sql, docs/DATABASE_STRUCTURE.md Create the migration file at `migrations/0.338.sql` (kolejna wersja po 0.337):
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). 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. AC-3 precondition: column definition prepared for migration

Fix BUG-2: Add is_cod to PaymentMethodRepository normalization and queries autoload/Domain/PaymentMethod/PaymentMethodRepository.php 1. In `normalizePaymentMethod(array $row)`: add `$row['is_cod'] = (int)($row['is_cod'] ?? 0);`
  1. 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.

  2. 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).

  3. 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.

  4. 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. php -l autoload/Domain/PaymentMethod/PaymentMethodRepository.php — no syntax errors. All explicit SQL SELECTs in this file now include is_cod. AC-3 + AC-4 precondition: repository returns is_cod field

Fix BUG-2: Add is_cod switch to admin payment method form autoload/admin/Controllers/ShopPaymentMethodController.php In `buildFormViewModel()`:
  1. Add 'is_cod' => (int)($paymentMethod['is_cod'] ?? 0) to the $data array.

  2. Add a switch field after the status field:

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:

'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. 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. AC-3 satisfied: admin can set is_cod flag on any payment method

Fix BUG-2: Use is_cod flag instead of hardcoded payment_id == 3 in OrderRepository autoload/Domain/Order/OrderRepository.php In `createFromBasket()`, at lines 817-820, replace the hardcoded check:
// BEFORE:
if ($payment_id == 3) {
    $this->updateOrderStatus($order_id, 4);
    $this->insertStatusHistory($order_id, 4, 1);
}

With:

// 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:

$payment_method = ( new \Domain\PaymentMethod\PaymentMethodRepository( $this->db ) )->findActiveById( (int)$payment_id );

So $payment_method['is_cod'] is available without any additional DB query. php -l autoload/Domain/Order/OrderRepository.php — no syntax errors. Confirm the old $payment_id == 3 no longer exists in createFromBasket(). AC-4 satisfied: COD status promotion is driven by is_cod flag, not hardcoded ID

Run the database migration on production server 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).
Claude will verify the code changes are in place. The DB migration must be confirmed by you. Type "done" when migration and admin flag set ## 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
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

<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>
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