docs: complete project research for stabilization milestone

Add STACK, FEATURES, ARCHITECTURE, PITFALLS, and SUMMARY research files
covering the pomysloweprezenty.pl bugfix/stabilization milestone scope.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-03-10 21:31:52 +01:00
parent 951b7dbe4c
commit 7501ecbcd7
5 changed files with 1351 additions and 0 deletions

View File

@@ -0,0 +1,437 @@
# Architecture Research
**Domain:** Custom PHP e-commerce (autorski CMS/framework)
**Researched:** 2026-03-10
**Confidence:** HIGH — based on direct codebase inspection
## Standard Architecture
### System Overview
```
┌─────────────────────────────────────────────────────────────────┐
│ Entry Points │
│ ┌──────────┐ ┌──────────┐ ┌──────────┐ ┌──────────────┐ │
│ │ index.php│ │ ajax.php │ │ api.php │ │ admin/ │ │
│ │ (frontend│ │ (AJAX │ │ (REST │ │ ajax.php │ │
│ │ pages) │ │ frontend)│ │ API) │ │ (admin ops) │ │
│ └────┬─────┘ └────┬─────┘ └────┬─────┘ └──────┬───────┘ │
└───────┼─────────────┼─────────────┼────────────────┼────────────┘
│ │ │ │
┌───────▼─────────────▼─────────────▼────────────────▼────────────┐
│ Routing / Controller Layer │
│ ┌──────────────────────┐ ┌───────────────────────────────────┐ │
│ │ front\App::route() │ │ api\ApiRouter::handle() │ │
│ │ (GET param routing) │ │ (endpoint + action routing) │ │
│ │ + old front\controls│ │ + admin Controllers\*Controller │ │
│ └──────────┬───────────┘ └──────────────┬────────────────────┘ │
└─────────────┼────────────────────────────┼──────────────────────┘
│ │
┌─────────────▼────────────────────────────▼──────────────────────┐
│ Domain / Service Layer │
│ ┌────────────┐ ┌────────────┐ ┌──────────────┐ ┌────────────┐ │
│ │ Domain\ │ │ Domain\ │ │ Domain\ │ │ Domain\ │ │
│ │ Order │ │ Basket │ │ Product │ │ Client │ │
│ │ (Repo + │ │ (Calculator│ │ (Repo) │ │ (Repo) │ │
│ │ Service) │ │ ) │ │ │ │ │ │
│ └────────────┘ └────────────┘ └──────────────┘ └────────────┘ │
│ ┌────────────┐ ┌────────────┐ ┌──────────────┐ ┌────────────┐ │
│ │ Domain\ │ │ Domain\ │ │ Domain\ │ │ Domain\ │ │
│ │ Coupon │ │ Promotion │ │ Transport │ │ Payment │ │
│ │ (Repo) │ │ (Repo) │ │ Method(Repo)│ │ Method │ │
│ └────────────┘ └────────────┘ └──────────────┘ └────────────┘ │
│ ┌────────────┐ ┌────────────┐ ┌──────────────┐ ┌────────────┐ │
│ │ Domain\ │ │ Domain\ │ │ Domain\ │ │ Domain\ │ │
│ │ Category │ │ Article │ │ Integrations│ │ CronJob │ │
│ │ (Repo) │ │ (Repo) │ │ (Repo+Logger│ │ (Processor│ │
│ └────────────┘ └────────────┘ └──────────────┘ └────────────┘ │
└─────────────────────────────────┬───────────────────────────────┘
┌─────────────────────────────────▼───────────────────────────────┐
│ Shared Utilities │
│ ┌───────────────┐ ┌───────────┐ ┌─────────┐ ┌───────────┐ │
│ │ Shared\Helpers│ │ Shared\ │ │ Shared\ │ │ Shared\ │ │
│ │ \Helpers (S::)│ │ Tpl\Tpl │ │ Cache │ │ Email │ │
│ │ (global proxy)│ │ (templating)│ │ │ │ │ │
│ └───────────────┘ └───────────┘ └─────────┘ └───────────┘ │
└─────────────────────────────────┬───────────────────────────────┘
┌─────────────────────────────────▼───────────────────────────────┐
│ Data Layer │
│ ┌──────────────────────────┐ ┌──────────────────────────────┐ │
│ │ Medoo (query builder) │ │ RedBeanPHP (ORM, legacy) │ │
│ │ $mdb — primary │ │ R:: — used in index.php │ │
│ └──────────────────────────┘ └──────────────────────────────┘ │
│ ┌────────────────────────────────────────────────────────────┐ │
│ │ MySQL Database │ │
│ │ Tables: pp_* prefix (products, orders, clients, etc.) │ │
│ └────────────────────────────────────────────────────────────┘ │
└─────────────────────────────────────────────────────────────────┘
```
### Component Responsibilities
| Component | Responsibility | Notes |
|-----------|----------------|-------|
| `index.php` | Front-end bootstrap: autoload, session, routing, output post-processing | Also injects GTM, Facebook Pixel, Open Graph meta, WebP conversion, lazy loading |
| `ajax.php` | Front-end AJAX requests from browser (basket ops, client actions) | Separate entry point; shares session with index.php |
| `api.php` | REST API entry point for external integrations (Apilo, ShopPro) | Key-authenticated via `X-API-Key` header |
| `admin/` | Admin panel: own bootstrap, own ajax.php, own templates | Separate routing from frontend |
| `front\App` | Front-end router: dispatches GET params to Controllers or legacy `front\controls\*` | Dual dispatch: new DI controllers + legacy static classes |
| `api\ApiRouter` | REST API router: resolves endpoint+action to controller; handles auth | Clean DI pattern; newer code |
| `Domain\*` | Business logic: each subdomain has Repository (DB access) and optionally Service | Repository = data access; Service = orchestration |
| `Domain\Basket\BasketCalculator` | Stateless price/stock calculation for basket positions | Falls back to `$GLOBALS['mdb']` when no DI provided |
| `Domain\Order\OrderAdminService` | Order management: listing, status change, creation | Orchestrates OrderRepo + ProductRepo + TransportRepo + CronJob |
| `Domain\Integrations` | Apilo/ShopPro integration logic + logging | `ApiloLogger.php` writes to `logs/` |
| `Domain\CronJob` | Async job queue: Processor + Repository + Types | Used by OrderAdminService for background tasks |
| `Shared\Helpers\Helpers` | Global utility class aliased as `\S::` | Session, GET/POST access, error messages, decimal normalization |
| `Shared\Tpl\Tpl` | Template renderer: wraps PHP include with variable injection | Looks for template in `templates_user/` first (theme override), then `templates/` |
| `front\controls\*` | Legacy static controller classes (pre-DI era) | Still in use for many modules; intermixed with new Controllers |
| `front\factory\*` | Legacy static factory classes (Settings, Languages, ShopProduct, etc.) | Called from index.php directly; global scope dependencies |
| `templates/` | PHP template files for frontend views | One file per view; variables injected by Tpl or front\controls |
| `admin/templates/` | PHP template files for admin panel views | Same pattern as frontend |
| `libraries/` | Vendored third-party code (Medoo, RedBeanPHP, PHPMailer) | Not Composer-managed; manual includes |
## Recommended Project Structure
The actual project structure (document as-is, since we are stabilizing not redesigning):
```
/
├── index.php # Frontend entry point (routing, bootstrap, output post-processing)
├── ajax.php # Frontend AJAX entry point
├── api.php # REST API entry point
├── config.php # Database credentials, site config
├── cron.php # Cron job entry point
├── autoload/
│ ├── Domain/ # Business domain objects
│ │ ├── Basket/ # BasketCalculator (stateless price/stock logic)
│ │ ├── Order/ # OrderRepository + OrderAdminService
│ │ ├── Product/ # ProductRepository
│ │ ├── Client/ # ClientRepository
│ │ ├── Category/ # CategoryRepository
│ │ ├── Coupon/ # CouponRepository
│ │ ├── Promotion/ # PromotionRepository
│ │ ├── Transport/ # TransportRepository
│ │ ├── PaymentMethod/ # PaymentMethodRepository
│ │ ├── Integrations/ # IntegrationsRepository + ApiloLogger
│ │ ├── CronJob/ # CronJobProcessor + Repository + Type
│ │ └── ... # Article, Banner, Newsletter, Pages, etc.
│ ├── Shared/ # Cross-cutting utilities
│ │ ├── Helpers/ # Helpers.php (aliased as \S::)
│ │ ├── Tpl/ # Tpl.php (template engine)
│ │ ├── Cache/ # Caching layer
│ │ ├── Email/ # Email sending wrapper
│ │ ├── Html/ # HTML utilities
│ │ └── Image/ # Image processing (WebP generation)
│ ├── api/ # REST API layer
│ │ ├── ApiRouter.php # Route + authenticate incoming API requests
│ │ └── Controllers/ # OrdersApiController, ProductsApiController, etc.
│ ├── admin/ # Admin panel layer
│ │ ├── Controllers/ # One controller per admin section
│ │ ├── Support/ # TableListRequestFactory, Forms
│ │ └── ViewModels/ # ViewModel classes for admin views
│ └── front/ # Frontend layer
│ ├── App.php # Frontend router
│ ├── Controllers/ # New DI-style frontend controllers
│ ├── Views/ # View rendering classes (Articles, ShopCategory, etc.)
│ ├── controls/ # Legacy static controller classes
│ └── factory/ # Legacy static factory classes
├── templates/ # Frontend PHP templates (Tpl::view target)
├── templates_user/ # Theme overrides (checked first by Tpl)
├── admin/
│ ├── index.php # Admin panel entry point
│ ├── ajax.php # Admin AJAX entry point
│ └── templates/ # Admin PHP templates
├── libraries/ # Vendored: Medoo, RedBeanPHP, PHPMailer
├── plugins/ # Hook files (special-actions-end.php etc.)
├── logs/ # Integration logs (Apilo etc.)
└── upload/ # User-uploaded files
```
### Structure Rationale
- **`autoload/Domain/`:** Contains business logic organized by subdomain. Newer code follows Repository pattern with explicit DB injection. This is where all bug fixes to business logic should happen.
- **`autoload/Shared/`:** Cross-domain utilities. `Helpers.php` / `\S::` is a global façade used everywhere — changing it breaks everything.
- **`autoload/front/controls/` vs `autoload/front/Controllers/`:** Two generations of frontend controllers coexist. Old = static classes; new = DI classes instantiated in `App::getControllerFactories()`. Bug fixes should follow the pattern of the code being fixed, not force a migration.
- **`templates/` + `templates_user/`:** Tpl looks in `templates_user/` first, enabling theme overrides without touching core templates.
## Architectural Patterns
### Pattern 1: Repository Pattern (newer code)
**What:** Domain objects access the database only through a Repository class. Controllers receive the Repository via constructor injection.
**When to use:** All new code and any refactoring of Domain layer code. The ApiRouter and newer admin Controllers follow this pattern.
**Trade-offs:** Clean testability, explicit dependencies. Downside: some Repositories still fall back to `$GLOBALS['mdb']` when the caller does not inject — this is a known partial migration.
**Example:**
```php
// Controller receives repo via constructor
class OrdersApiController
{
public function __construct(OrderAdminService $service, OrderRepository $orders) { ... }
}
// ApiRouter wires dependencies explicitly
'orders' => function () use ($db) {
$orderRepo = new OrderRepository($db);
$service = new OrderAdminService($orderRepo, ...);
return new OrdersApiController($service, $orderRepo);
},
```
### Pattern 2: Legacy Static Factory (older code)
**What:** Static classes with no constructor injection. Database and session accessed via globals (`$GLOBALS['mdb']`, `$_SESSION`).
**When to use:** Only when modifying existing legacy code where a full DI migration is out of scope for a bugfix. Do not introduce new static factories.
**Trade-offs:** Fast to write, zero boilerplate. Impossible to unit test in isolation. Hidden dependencies make tracing bugs harder.
**Example:**
```php
// Called directly from index.php / front\controls\*
$settings = \front\factory\Settings::settings_details();
$product = \front\factory\ShopProduct::product_details(\S::get('product'), $lang_id);
```
### Pattern 3: Template Rendering via Tpl::view()
**What:** Controller builds a data array, passes it to `Tpl::view('path/to/template', $data)`. Template receives variables as object properties (`$this->varName`).
**When to use:** All frontend and admin template rendering. Already universal.
**Trade-offs:** Simple. No caching at template level. Template override via `templates_user/` is the only customisation mechanism.
**Example:**
```php
return \Shared\Tpl\Tpl::view('shop-product/product', [
'product' => $product,
'settings' => $settings,
'lang_id' => $lang_id,
]);
```
## Data Flow
### Frontend Page Request Flow
```
Browser GET /?module=X&action=Y (or SEO URL → rewritten via pp_routes table)
|
index.php
├── Bootstrap: autoload, config, Medoo, RedBeanPHP, PHPMailer
├── Session init + IP check
├── Language resolution (session cache)
├── Settings load (front\factory\Settings)
├── Redirect check (pp_redirects table)
├── Route match (pp_routes table → rewrites $_GET)
└── front\App::route()
├── New Controllers (DI): getControllerFactories()[$module]()->$action()
├── Legacy: front\controls\$Module::$action()
└── Special cases: product view, category view, article view
|
Tpl::view('template-name', $data)
|
templates/*.php (or templates_user/*.php override)
|
Output assembled as string
|
index.php post-processing (DOM manipulation):
├── Cookie banner injection
├── GTM / Facebook Pixel injection
├── Open Graph meta injection
├── Footer CSS/JS reordering
├── WebP image conversion
└── Lazy loading attributes
|
echo $html
```
### AJAX Request Flow (frontend basket / client ops)
```
Browser XHR → ajax.php
├── Bootstrap: autoload, config, Medoo, session
└── Dispatches to handler (basket ops, client login/register, etc.)
|
front\controls\* or Domain\* logic
|
JSON response or HTML fragment
```
### REST API Request Flow (Apilo, ShopPro)
```
External service → api.php
├── Bootstrap: autoload, config, Medoo
└── api\ApiRouter::handle()
├── Authenticate (X-API-Key header vs pp_settings.api_key)
├── Resolve endpoint → Controller factory
└── Call $controller->$action()
|
Domain\*Repository (injected via factory)
|
JSON response via ApiRouter::sendSuccess() / sendError()
```
### Order Placement Flow (critical path)
```
Basket (session: $_SESSION['basket'])
|
Basket AJAX ops (add/remove/update quantity)
├── BasketCalculator::summaryPrice() [reads ProductRepository]
├── BasketCalculator::checkProductQuantityInStock()
└── Coupon / Promotion discount application
|
Checkout: address form → payment/transport selection
|
Order creation
├── OrderRepository::create() [writes to DB]
├── Stock decrement on products
├── Coupon usage increment
└── CronJob queue (email notifications, integration sync)
|
Payment gateway redirect / callback
|
Order status update → admin panel
```
### Admin Panel Request Flow
```
Browser → admin/index.php
├── Bootstrap (own autoload, config, Medoo, session auth check)
└── Dispatch to admin\Controllers\*Controller
├── Uses Domain\*Repository (injected or via globals)
├── admin\Support\TableListRequestFactory (list views)
└── Tpl::view('admin/templates/...', $data)
```
### Key Data Flows
1. **Basket state:** Stored entirely in `$_SESSION['basket']` as an array. All price recalculation reads from DB via ProductRepository on every request — no basket cache.
2. **Settings:** Loaded once from DB (`front\factory\Settings`), cached in session. Admin changes require session clear or expiry to propagate.
3. **Language / translations:** Loaded from DB, cached per `lang_id` in session. Multiple language support via `lang_id` parameter.
4. **Integration sync (Apilo/ShopPro):** Triggered via CronJob queue or direct REST API calls. Logged to `logs/` via `ApiloLogger`.
## Scaling Considerations
| Scale | Architecture Adjustments |
|-------|--------------------------|
| 0-10k orders/month | Current monolith is adequate; no changes needed |
| 10k-100k orders/month | Session-based basket becomes a bottleneck under concurrent load; consider DB-backed sessions. Two DB clients (Medoo + RedBeanPHP) add memory overhead. |
| 100k+ orders/month | Full architectural review needed; not in scope for this project |
### Scaling Priorities (informational, not current concern)
1. **First bottleneck:** Session locking — PHP file sessions block concurrent requests from the same user. Fix: Redis/Memcached session handler.
2. **Second bottleneck:** The `pp_routes` table is queried on every frontend request. Fix: route cache.
## Anti-Patterns
### Anti-Pattern 1: Global State via $GLOBALS and \S::
**What people do:** New code calls `$GLOBALS['mdb']` or `\S::get_session()` inside Domain objects instead of receiving dependencies via constructor.
**Why it's wrong:** Invisible dependencies make bugs hard to trace. A Domain object that calls `$GLOBALS['mdb']` breaks if instantiated outside a web request context (e.g., CLI cron, future tests).
**Do this instead:** Always inject `$db` (Medoo instance) into Repository constructors. `BasketCalculator` has a known instance of this problem — it falls back to `$GLOBALS['mdb']` when `$productRepo` is null. Fix the caller to always pass the repo, never let the fallback trigger.
### Anti-Pattern 2: Mixing Two DB Libraries for the Same Operation
**What people do:** Some code uses RedBeanPHP (`R::`) and some uses Medoo (`$mdb`) for overlapping data. `index.php` initialises both.
**Why it's wrong:** Two DB connections per request, inconsistent query patterns, and transaction boundaries that don't span both libraries.
**Do this instead:** New Domain code uses Medoo exclusively via Repository pattern. Avoid writing new RedBeanPHP code. Do not migrate existing RedBeanPHP code during a bugfix — that is a refactor, not a fix.
### Anti-Pattern 3: Fixing Bugs by Patching Templates
**What people do:** Logic bugs (wrong price, wrong stock check) get "fixed" by adding conditional checks inside `templates/*.php` files.
**Why it's wrong:** Business logic in templates is invisible to other callers (AJAX handlers, API controllers, cron jobs). The same bug will surface in another entry point.
**Do this instead:** Fix logic in the Domain layer (Repository or Calculator class). Templates should only format and display data, never compute it.
### Anti-Pattern 4: Bypassing Basket Session Structure
**What people do:** Direct `$_SESSION['basket']` manipulation in templates or controls without going through `BasketCalculator` or the basket AJAX handler.
**Why it's wrong:** The basket array has a specific structure expected by `BasketCalculator::summaryPrice()` and `checkProductQuantityInStock()`. Ad-hoc writes corrupt it silently.
**Do this instead:** All basket mutations go through the AJAX handler (`ajax.php` → basket ops). Read basket state via `BasketCalculator` methods.
## Integration Points
### External Services
| Service | Integration Pattern | Notes |
|---------|---------------------|-------|
| Apilo | REST API (outbound) via `Domain\Integrations\IntegrationsRepository` + `ApiloLogger` | Credentials in config/settings. Logs failures to `logs/`. |
| ShopPro | REST API (outbound), same infrastructure as Apilo | Uses same IntegrationsRepository pattern |
| tpay (payment) | Payment gateway redirect + callback to `tpay.txt` / dedicated handler | Payment callback must be idempotent — order status can be set multiple times |
| Facebook Pixel | Script injected in `index.php` post-processing via DOM manipulation | Settings-controlled; no code change needed to enable/disable |
| Google Tag Manager | Script injected in `index.php` post-processing | Same pattern as Facebook Pixel |
| PHPMailer | Email sending via `Shared\Email` wrapper | SMTP credentials in config |
### Internal Boundaries
| Boundary | Communication | Notes |
|----------|---------------|-------|
| `frontend ↔ Domain\Basket` | Direct PHP call via `BasketCalculator` + Session | No HTTP boundary; same process |
| `admin Controllers ↔ Domain\Order` | Direct PHP call via `OrderAdminService` (injected) | Clean DI; safe to modify |
| `api Controllers ↔ Domain\Order` | Direct PHP call via `OrderAdminService` (injected) | Same service as admin — changes affect both |
| `frontend ↔ front\factory\*` | Static method calls with global/session state | Legacy; avoid changing factory internals during bugfix |
| `CronJob ↔ Domain\*` | `CronJobProcessor` reads queue, calls Domain services | Async; errors here are silent unless logged |
| `templates ↔ Tpl::view()` | PHP include with `$this->var` variable injection | Template has access to all injected vars plus global scope |
## Bugfix Work Organization
### Safe Change Zones (low blast radius)
- `Domain\*Repository` — isolated DB queries; changes affect only callers of that repo method
- `Domain\Order\OrderAdminService` — orchestration only; well-bounded
- `Domain\Basket\BasketCalculator` — stateless calculations; verifiable by input/output
- `api\Controllers\*` — only affect REST API consumers (Apilo, ShopPro)
- `admin\Controllers\*` — only affect admin panel
### Risky Change Zones (high blast radius)
- `Shared\Helpers\Helpers` / `\S::` — used everywhere; any change propagates across all entry points
- `front\factory\*` static classes — called from index.php, templates, and controls; session-cached results
- `index.php` post-processing section — affects every frontend page response
- `BasketCalculator::calculateBasketProductPrice()` — core pricing logic; used in basket display, order creation, and potentially API
### Bugfix Protocol for This Project
1. **Identify the entry point** where the bug manifests (frontend page / AJAX / API / admin / cron)
2. **Trace to Domain layer** — find the Repository or Calculator responsible for the wrong data
3. **Fix in Domain layer** — never in template, never in entry point bootstrap
4. **Check all callers**`BasketCalculator`, `OrderAdminService`, API controllers, and AJAX handlers may all call the same Domain method
5. **Test affected entry points** — if a basket price bug is fixed, verify: frontend basket display, AJAX basket update, checkout summary, order creation
6. **Do not refactor while fixing** — resist the temptation to migrate legacy static code to DI during a bugfix; that is a separate milestone
### Component Fix Priority Order
Dependencies flow upward — fix foundational components before their consumers:
```
1. Domain\*Repository (data access — fix data integrity bugs first)
2. Domain\Basket\Calculator (pricing logic — depends on ProductRepository)
3. Domain\Order\Service (orchestration — depends on Basket, Product, Transport)
4. front\controls\* / Controllers (display logic — depends on Domain)
5. templates\* (presentation — fix only display formatting here)
```
## Sources
- Direct codebase inspection: `/autoload/Domain/`, `/autoload/front/`, `/autoload/api/`, `/autoload/Shared/`, `index.php`, `ajax.php`, `api.php` — HIGH confidence
- Architecture patterns derived from code structure, not external references — HIGH confidence for this codebase
---
*Architecture research for: PomyslowePrezenty.pl — custom PHP e-commerce*
*Researched: 2026-03-10*

View File

@@ -0,0 +1,212 @@
# Feature Research
**Domain:** Custom PHP e-commerce shop (bugfix/stabilization milestone)
**Researched:** 2026-03-10
**Confidence:** HIGH (based on direct codebase analysis + MEDIUM for general e-commerce patterns)
---
## Context: This Is a Stabilization Milestone
This is not a "what to build" research — it is "what must work reliably."
The existing feature set is already deployed and serving real customers.
The goal is to identify **which quality attributes are table stakes** (broken = users leave / revenue lost)
vs **differentiators** (works better than expected = competitive edge)
vs **anti-features** (scope creep traps during bugfixing).
---
## Feature Landscape
### Table Stakes (Users Expect These — Broken = Revenue Loss)
Features that must work correctly or customers abandon and never return.
In a live production shop, these are also legal/financial obligations.
| Feature | Why Expected | Complexity | Notes |
|---------|--------------|------------|-------|
| Cart price correctness | Customers trust displayed price = charged price. Discrepancy = chargebacks, complaints | HIGH | BasketCalculator has complex coupon+promo+discount type interaction (types 1, 3). Float precision risk identified in code (`round(..., 2)` added as mitigation). |
| Coupon code applies correctly | Discounts are a purchase trigger. Wrong application = fraud or revenue leak | HIGH | CouponRepository mixes object/array access for coupon in BasketCalculator — dual-path code increases bug surface. `is_array($coupon)` vs `is_object($coupon)` branches throughout. |
| Inventory decrement on order | Overselling creates fulfillment crises. Stock must decrease atomically with order creation | HIGH | `createFromBasket()` decrements stock after inserting products — no transaction wrapping observed. Race condition risk under concurrent orders for low-stock items. |
| Order number uniqueness | Duplicate order numbers break admin, accounting, fulfillment | MEDIUM | `generateOrderNumber()` uses MAX() + date prefix. No locking — concurrent submissions risk duplicate numbers. |
| Payment status accuracy | `paid=0` on a completed payment = fulfilled order without revenue. `paid=1` on failed payment = revenue given away | HIGH | Przelewy24 hash-based verification in `orderDetailsFrontend()`. Payment gateway webhook handling is critical path. |
| Delivery cost threshold (free delivery) | Customers rely on "free over X zł" promotions. Wrong calculation = broken promise | MEDIUM | `createFromBasket()` checks `$transport['delivery_free'] == 1 && $basket_summary >= $settings['free_delivery']`. Admin order edit has separate `recalculateTransportCost()` — consistency risk. |
| Order confirmation email | Customers need proof of purchase. Missing email = support tickets and distrust | MEDIUM | Email sent in `createFromBasket()`. `preg_replace` converts relative URLs — fragile, can produce malformed links. |
| Stock validation before checkout | Attempting to place order for out-of-stock items must be blocked or quantity adjusted | MEDIUM | `checkProductQuantityInStock()` exists but is session-based. Not re-validated at order creation time — gap between basket load and order submit. |
| Transport method + InPost point required | Attempting to proceed without selecting a pickup point for InPost/Orlen must be blocked | LOW | Frontend guards exist (`transport_method_inpost_check`). Backend must re-validate. |
| Hardcoded payment ID == 3 triggers status change | `payment_id == 3` (cash on delivery) auto-advances to status 4 — must match actual DB record | HIGH | Line 817 in OrderRepository: `if ($payment_id == 3)`. If the cash-on-delivery method ID changes in DB, this silently breaks. Magic number coupling. |
| Session-based basket integrity | Basket in session must remain consistent across page loads and AJAX updates | MEDIUM | All basket mutations go through session. Multiple AJAX endpoints update independently — partial-update risk if one fails. |
| Admin order summary recalculation | Admin editing an order must produce correct totals | MEDIUM | `calculateOrderSummaryByAdmin()` is separate from `BasketCalculator` — two independent price calculation paths. Divergence risk. |
| Status change emails reaching clients | Clients receive email on status transitions — shipped, cancelled, etc. | MEDIUM | `sendStatusChangeEmail()` uses `#sklep-zmiana-statusu-zamowienia` template loaded by name — silent failure if template missing. |
| Apilo/ShopPro sync reliability | Orders must sync to fulfilment integrations or warehouse gets out of sync | HIGH | CronJob queue introduced for async retry. Failure path is well-logged via `ApiloLogger`. Sync failures are queued — but cron must actually run. |
### Differentiators (Competitive Advantage — Above Baseline)
Features that, when working well, create trust and reduce support overhead.
| Feature | Value Proposition | Complexity | Notes |
|---------|-------------------|------------|-------|
| Coupon with per-category scope | Targeted promotions (e.g., 20% off category X only) | HIGH | Already implemented in `calculateBasketProductPrice()`. When working correctly, this is a business differentiator. Currently complex code = bug risk. |
| Promotion rules engine (5 types) | Complex conditional discounts (cheapest product free, cross-category) | HIGH | `PromotionRepository` defines 5 condition types. Complex logic = more testing surface required. |
| InPost + Orlen Paczka integration | Polish market standard — convenience for customers | MEDIUM | MutationObserver + AJAX save for Orlen point. Fragile DOM observation pattern. |
| Apilo order status sync with retry queue | Orders don't silently fail to sync — DB-backed retry with CronJob | HIGH | Just implemented (recent commit). Key differentiator for operational reliability. |
| Admin: order product editing with stock correction | Admin can fix order contents and stock adjusts automatically | HIGH | `saveOrderProducts()` in `OrderAdminService` handles delta-based stock correction. Complex but high value. |
| Order notes for admin | Internal notes on orders without affecting client | LOW | `saveNotes()` method exists. Straightforward. |
| TrustMate review sync toggle | Sending orders to review platform | LOW | `toggleTrustmateSend()` exists. Low complexity, operational value. |
| Product custom fields on order | Gift messages, personalization captured per order line | LOW | `custom_fields` stored in `pp_shop_order_products`. Already works — just needs to render correctly. |
### Anti-Features (Commonly Requested, Often Problematic)
Things that seem helpful but introduce risk during a stabilization milestone.
| Feature | Why Requested | Why Problematic | Alternative |
|---------|---------------|-----------------|-------------|
| Rewrite price calculation to "clean" it up | BasketCalculator is complex and hard to follow | Rewrites during stabilization introduce new bugs. The current logic, however messy, has been running in production | Audit + document current logic first, fix specific bugs, do not rewrite |
| Add database transactions to all order operations | Race conditions exist — transactions seem like a fix | Medoo ORM wraps PDO but transaction handling needs audit. Wrapping incorrectly can cause silent data loss | Fix the specific race condition (order number generation) with a targeted `SELECT ... FOR UPDATE` or unique constraint, not a global rewrite |
| Move basket from session to database | "More reliable" | Breaks existing user flows, requires migration of all active sessions, adds DB load on every page | Session-based basket is fine for this shop's scale. Fix session mutation bugs instead |
| Real-time stock updates via WebSocket | "Modern" | Massively out of scope, changes infrastructure, solves a problem this shop doesn't have at current scale | Fix the race condition in stock decrement at order creation — that is the actual problem |
| Replace AJAX with fetch/async-await everywhere | Modernize JS | jQuery iCheck plugin has deep integration in basket UI. Migration mid-stabilization creates regression risk | Leave jQuery/AJAX as-is. Fix logic bugs, not technology |
| Add automated test suite from scratch | Good practice | Writing tests for untested legacy code is valuable long-term but is a separate project. During bugfix phase it delays actual fixes | Add tests only for the specific functions being fixed, as regression guards |
---
## Feature Dependencies
```
[Order creation — createFromBasket()]
└──requires──> [Basket price calculation — BasketCalculator::summaryPrice()]
└──requires──> [Coupon validation — CouponRepository::isAvailable()]
└──requires──> [Product data — ProductRepository::findCached()]
└──requires──> [Transport cost calculation]
└──requires──> [Free delivery threshold from settings]
└──requires──> [Stock decrement — ProductRepository::updateQuantity()]
└──requires──> [Order number generation — generateOrderNumber()]
└──requires──> [Email dispatch — Helpers::send_email()]
└──triggers──> [Apilo sync — OrderAdminService::syncApiloStatusIfNeeded()]
└──requires──> [CronJob queue running]
[Admin order edit — saveOrderByAdmin()]
└──requires──> [Transport cost recalculation — recalculateTransportCost()]
└──requires──> [Free delivery threshold — same as above]
└──requires──> [Order summary recalculation — calculateOrderSummaryByAdmin()]
NOTE: This is a DIFFERENT calculation path than BasketCalculator
[Payment confirmation — setOrderAsPaid()]
└──triggers──> [Apilo payment sync]
└──triggers──> [Status change to 1 then 4]
└──triggers──> [Status change email]
```
### Dependency Notes
- **Two independent price calculation paths** exist: `BasketCalculator::summaryPrice()` (frontend checkout) and `OrderRepository::calculateOrderSummaryByAdmin()` (admin edit). They must produce consistent results for the same order contents. This is the highest-priority consistency risk.
- **Coupon object vs array duality**: `BasketCalculator::calculateBasketProductPrice()` accepts coupon as either object or array and branches on `is_array($coupon)`. `createFromBasket()` passes it as object, admin-side code may pass it as array. Inconsistency risk.
- **Hardcoded payment ID 3**: `createFromBasket()` has `if ($payment_id == 3)` to auto-advance status for cash-on-delivery. This creates a hidden dependency between business logic and DB record IDs that will silently break if the record is modified.
- **Stock decrement not transactional**: `createFromBasket()` inserts order record, then decrements stock in a loop. If the process dies mid-loop, order exists but some stock was not decremented. Partial state is possible.
- **Apilo sync requires cron to run**: The retry queue added in recent commits only works if the CronJob system is being executed. If cron is misconfigured, syncs silently queue forever.
---
## MVP Definition (for this Stabilization Milestone)
### Fix First (P1 — Revenue and Trust Critical)
- [ ] **Price calculation correctness** — Audit coupon + promo + discount type combinations end-to-end. Confirm frontend checkout and admin edit produce identical totals.
- [ ] **Hardcoded payment_id == 3** — Replace magic number with a lookup or named constant derived from DB to prevent silent breakage.
- [ ] **Stock decrement race condition** — Investigate whether concurrent orders for the same low-stock product can oversell. Add `stock_0_buy` flag check at order-creation time, not just basket load.
- [ ] **Order number uniqueness under concurrency**`generateOrderNumber()` uses `MAX()` without locking. Evaluate adding a unique constraint or sequence mechanism.
- [ ] **Order confirmation email URL correctness** — The `preg_replace` for relative-to-absolute URL rewriting is fragile. Test with all image/link patterns in the email template.
### Fix Second (P2 — Operational Reliability)
- [ ] **Apilo sync cron verification** — Confirm CronJob is running and retry queue drains. Log failures visibly in admin panel.
- [ ] **Admin order edit transport cost consistency** — Verify `recalculateTransportCost()` matches what frontend shows at checkout. Inconsistency creates pricing disputes.
- [ ] **Status change email template existence guard**`Email::load_by_name()` must fail visibly (not silently) if template is missing.
- [ ] **InPost/Orlen point validation on backend** — Frontend guard exists but backend must independently validate that a pickup point was selected before accepting the order.
### Defer to Next Milestone (P3 — Nice to Have)
- [ ] **Automated test coverage** — Valuable but does not fix current bugs. Plan after stabilization.
- [ ] **Basket-to-DB migration** — Not a current pain point.
- [ ] **New promotions/coupon types** — Out of scope per PROJECT.md.
---
## Feature Prioritization Matrix
| Feature | User Value | Implementation Cost | Priority |
|---------|------------|---------------------|----------|
| Price calculation correctness | HIGH | HIGH | P1 |
| Hardcoded payment ID 3 | HIGH | LOW | P1 |
| Stock decrement race condition | HIGH | MEDIUM | P1 |
| Order number uniqueness | HIGH | LOW | P1 |
| Email URL rewriting correctness | HIGH | LOW | P1 |
| Apilo sync cron verification | MEDIUM | LOW | P2 |
| Admin/frontend price path consistency | HIGH | MEDIUM | P2 |
| Status email silent failure guard | MEDIUM | LOW | P2 |
| InPost backend validation | MEDIUM | LOW | P2 |
| Coupon object/array duality cleanup | MEDIUM | MEDIUM | P2 |
| Automated tests | LOW (short term) | HIGH | P3 |
**Priority key:**
- P1: Must fix in this milestone — blocks revenue or trust
- P2: Fix in this milestone if possible — operational reliability
- P3: Defer to next milestone
---
## Bug Category Map (Custom PHP E-Commerce Patterns)
Drawn from codebase analysis. These are the actual risk areas found:
### Category 1: Price Calculation Bugs (Severity: CRITICAL)
Multiple discount mechanisms (promo price, coupon percentage, coupon fixed amount, promotion rules)
interact in `BasketCalculator::calculateBasketProductPrice()`. The logic branches on 6+ conditions.
Risk: a combination of coupon type + discounted product + category filter produces wrong final price.
### Category 2: State Consistency Bugs (Severity: HIGH)
Two separate price calculation implementations (checkout vs admin edit) can diverge.
Session basket state can become inconsistent if an AJAX call fails mid-sequence.
Order summary is stored in DB at creation time — if admin edits order without triggering recalc, totals drift.
### Category 3: Integration Reliability Bugs (Severity: HIGH)
Apilo sync failures were previously silent — now queued. Cron must run.
Payment gateway (Przelewy24) callback must arrive and be processed correctly — hash-based lookup.
Magic number coupling (`payment_id == 3`) is a category of "business rule baked into data IDs."
### Category 4: Concurrency Bugs (Severity: MEDIUM)
Order number generation: `MAX()` without lock — duplicate possible under simultaneous submissions.
Stock decrement: not transactional — oversell possible under simultaneous orders for same product.
### Category 5: Validation Gap Bugs (Severity: MEDIUM)
Stock validated at basket load, not re-validated at order submission.
InPost/Orlen point validated frontend only — backend re-check is a defensive necessity.
Form fields have `required` conditionally removed in template — backend validation must be independent.
### Category 6: Template/Email Rendering Bugs (Severity: LOW-MEDIUM)
Email template uses `preg_replace` for URL rewriting — edge cases with unusual paths.
Email template referenced by name — missing template produces silent failure.
Admin templates mixing Polish language strings in PHP business logic and templates.
---
## Sources
- Direct codebase analysis: `autoload/Domain/Basket/BasketCalculator.php`
- Direct codebase analysis: `autoload/Domain/Order/OrderRepository.php`
- Direct codebase analysis: `autoload/Domain/Order/OrderAdminService.php`
- Direct codebase analysis: `autoload/Domain/Coupon/CouponRepository.php`
- Direct codebase analysis: `autoload/Domain/Promotion/PromotionRepository.php`
- Direct codebase analysis: `templates/shop-basket/basket.php`
- Direct codebase analysis: `templates/shop-basket/address-form.php`
- PROJECT.md milestone context: bugfix/stabilization, no new features, production shop
- Confidence: HIGH for codebase-derived findings, MEDIUM for general e-commerce patterns
---
*Feature research for: pomysloweprezenty.pl — bugfix/stabilization milestone*
*Researched: 2026-03-10*

View File

@@ -0,0 +1,276 @@
# Pitfalls Research
**Domain:** Custom PHP e-commerce shop — bugfix/stabilization milestone
**Researched:** 2026-03-10
**Confidence:** HIGH (based on direct codebase analysis + established PHP e-commerce patterns)
---
## Critical Pitfalls
### Pitfall 1: Fixing Price Logic Without Tracing All Callers
**What goes wrong:**
`BasketCalculator::calculateBasketProductPrice()` and `BasketCalculator::summaryPrice()` are called from at least three places: `ShopBasketController` (live basket), `OrderRepository::createFromBasket()` (order creation), and `OrderRepository::calculateOrderSummaryByAdmin()` (admin recalculation). Fixing a discount/coupon bug in one path while leaving other paths untouched causes inconsistent totals — the basket shows the correct price but the saved order total differs, or vice versa.
**Why it happens:**
Price logic is spread across Domain, Controller, and Repository layers. A developer traces the bug to one visible symptom (e.g., wrong basket total) and fixes that call site without realizing `calculateOrderSummaryByAdmin` computes the summary independently (without coupon) for admin edits.
**How to avoid:**
Before touching any price/discount logic, map every call site with `grep -rn "calculateBasketProductPrice\|summaryPrice\|calculateOrderSummaryByAdmin"`. Make all call sites consistent. If recalculation semantics differ between basket and admin contexts, document that explicitly.
**Warning signs:**
- Order summary in DB differs from basket total shown at checkout.
- Admin editing an order resets the total to a different value than what was charged.
- Coupon discount appears on order confirmation email but not in `pp_shop_orders.summary`.
**Phase to address:** Phase 1 (basket/order bugfix). Must be the first thing audited.
---
### Pitfall 2: Double Coupon Usage on Retry / Page Reload
**What goes wrong:**
`OrderRepository::createFromBasket()` increments coupon usage count (`CouponRepository::incrementUsedCount`) and then separately calls `$coupon->set_as_used()` for one-time coupons (lines 722 and 793-795). If order creation fails partway through (e.g., email fails, DB insert error) or the user reloads the confirmation page, the coupon counter can be incremented without a valid order existing.
**Why it happens:**
There is no database transaction wrapping the order insert + coupon update + stock decrement operations in `createFromBasket()`. Each DB write is fire-and-forget via Medoo.
**How to avoid:**
Wrap the entire `createFromBasket()` flow in a MySQL transaction (`START TRANSACTION` / `COMMIT` / `ROLLBACK`). At minimum, check whether the coupon was already marked as used before calling `incrementUsedCount`. Add an idempotency guard at the basket POST endpoint (e.g., check session token consumed).
**Warning signs:**
- `used_count` in coupon table is higher than the number of orders using that coupon.
- Customers report coupon no longer valid after a failed checkout.
**Phase to address:** Phase 1 (order/payment bugfix).
---
### Pitfall 3: Race Condition in Stock Decrement
**What goes wrong:**
Stock is decremented in `createFromBasket()` with two sequential raw UPDATE calls: one for variant products (`parent_id`) and one for base products. There is no locking (`SELECT ... FOR UPDATE`). Under simultaneous orders for the same product, two requests can both pass the `checkProductQuantityInStock()` check and both decrement stock — potentially reaching -1 before the safety clamp `UPDATE pp_shop_products SET quantity = 0 WHERE quantity < 0` runs.
**Why it happens:**
Custom ORM (Medoo) makes it easy to write simple updates. Transaction + row-level locking requires dropping down to raw `$db->query()` calls, which is less convenient, so it was never done.
**How to avoid:**
Wrap the stock-decrement step in a transaction with `SELECT quantity FROM pp_shop_products WHERE id = ? FOR UPDATE`. Alternatively, use an atomic `UPDATE pp_shop_products SET quantity = GREATEST(0, quantity - ?) WHERE id = ?` without a prior read.
**Warning signs:**
- `quantity` column reaches -1 or -2 in `pp_shop_products`.
- Two orders exist for a product where only 1 was in stock.
**Phase to address:** Phase 1 (order creation bugfix).
---
### Pitfall 4: Payment Webhook Replay Attacks / Idempotency
**What goes wrong:**
Both Przelewy24 and Tpay callbacks call `setOrderAsPaid()` without checking whether the order is already paid. Przelewy24 additionally checks `$order['status'] == 0`, but Tpay's `paymentStatusTpay()` only checks `$order['id']` — no status guard. A replay of the webhook (which payment providers do on timeout) triggers `setOrderAsPaid()` again, sending a second confirmation email and potentially double-counting Apilo sync.
**Why it happens:**
Webhook handlers were written to be simple. "Check that order exists, mark as paid" felt sufficient. The idempotency requirement of webhooks is often underestimated.
**How to avoid:**
Add an early-return guard in each payment callback: `if ((int)$order['paid'] === 1) { echo 'OK'; exit; }`. Apply this before any state-changing call.
**Warning signs:**
- Customers receive two order confirmation emails.
- `pp_apilo_logs` contains duplicate `payment_sync` entries for the same order ID.
- Apilo sync queue (`pp_cron_jobs`) has two `APILO_SYNC_PAYMENT` records for the same order.
**Phase to address:** Phase 1 (payment bugfix). Tpay handler is the most exposed — fix first.
---
### Pitfall 5: Global `$mdb` / `$settings` Dependency Breaking Across Context Boundaries
**What goes wrong:**
Many domain classes fall back to `$GLOBALS['mdb']` when no `$db` is injected (e.g., `BasketCalculator::summaryPrice()` line 40, `BasketCalculator::checkProductQuantityInStock()` line 88). `OrderAdminService::sendOrderToApilo()` uses `global $mdb` directly (line 397). When these are called from the API router or from a cron job context where globals are set up differently, they silently receive `null`/`false` from Medoo and return wrong data instead of throwing.
**Why it happens:**
The fallback was added to avoid refactoring all call sites during migration to the Repository pattern. The intent was temporary scaffolding that became permanent.
**How to avoid:**
Never add new code that falls back to globals. When fixing bugs in methods that use globals, pass the dependency explicitly. Do not refactor all usages at once (too risky on production) — fix only the code path touched by the bug being addressed.
**Warning signs:**
- Basket total is 0.00 in some contexts but correct in others.
- API endpoints return empty product lists without error.
- Cron jobs complete without logging errors but produce no side effects.
**Phase to address:** Every phase. Flag in code review.
---
### Pitfall 6: Fixing a Bug Without Verifying the Session State Machine
**What goes wrong:**
The basket (`$_SESSION['basket']`), coupon (`$_SESSION['coupon']`), and transport choice (`$_SESSION['basket-transport-method-id']`) are stored separately in session. Fixes that touch one without the others can leave the session in an inconsistent state — e.g., a coupon referencing a category discount stays in session after the product triggering it is removed, or the transport cost is not recalculated after a basket change.
**Why it happens:**
Each basket action (add, remove, change quantity) calls `findPromotion()` to reapply promotion discounts but does not always re-evaluate coupon eligibility or transport free-delivery threshold. Developers reading one controller action miss that sibling actions handle it differently.
**How to avoid:**
Before fixing any basket action, read all basket controller methods as a group: `basketRemoveProduct`, `basketIncreaseQuantityProduct`, `basketDecreaseQuantityProduct`, `basketChangeQuantityProduct`, `addProductToBasket`. Confirm that every mutation calls the same set of recalculation steps. Any divergence between methods is a bug vector.
**Warning signs:**
- Transport cost in basket differs from what is charged in the order.
- Coupon discount persists in basket after the qualifying product was removed.
- Order summary does not match basket summary on confirmation page.
**Phase to address:** Phase 1 (basket bugfix).
---
### Pitfall 7: Fragile Order Number Generation Under Concurrency
**What goes wrong:**
`generateOrderNumber()` runs a raw `SELECT MAX(...)` on `pp_shop_orders` filtered by the current month, then returns `MAX + 1`. Two simultaneous order submissions can both read the same MAX and generate duplicate order numbers. The `number` column does not appear to have a UNIQUE index constraint.
**Why it happens:**
Sequential number generation was implemented correctly for single-threaded use. Concurrency at checkout was not considered.
**How to avoid:**
Do not change this logic during a bugfix phase — it is a foundational structural issue requiring a migration (add UNIQUE index or replace with auto-increment + format). Document it as a known risk. As a minimal mitigation, wrap `generateOrderNumber()` in a transaction with `GET_LOCK()` to prevent concurrent execution.
**Warning signs:**
- Two orders with identical `number` values in `pp_shop_orders`.
- Apilo sync failing with "duplicate order number" error.
**Phase to address:** Phase 2 (stabilization). Too risky to change in Phase 1 without a migration plan.
---
### Pitfall 8: Apilo `sendOrderToApilo()` Deletes the Original Order Before Confirming New One
**What goes wrong:**
`OrderAdminService::sendOrderToApilo()` (lines 476-497) clones the order rows into new DB rows, then deletes the originals. There is no transaction. If the INSERT succeeds but the DELETE fails (or vice versa), or if the new order INSERT fails silently, the order data can be lost or duplicated. The column list is built dynamically from `INFORMATION_SCHEMA`, which is fragile if column names change.
**Why it happens:**
The "resend to Apilo" feature was implemented as a workaround for Apilo's requirement to clear the `apilo_order_id` and re-send. The clone-and-delete approach works in isolation but is not atomic.
**How to avoid:**
Do not change this logic during bugfix unless it is the direct cause of a reported bug. If touching it, wrap in a transaction. Before deleting originals, verify the new IDs are non-zero.
**Warning signs:**
- Order disappears from admin panel after clicking "resend to Apilo".
- `pp_shop_orders` has orphaned rows with no matching `pp_shop_order_products`.
**Phase to address:** Phase 2 (stabilization). Flag as high-risk code.
---
## Technical Debt Patterns
| Shortcut | Immediate Benefit | Long-term Cost | When Acceptable |
|----------|-------------------|----------------|-----------------|
| `$GLOBALS['mdb']` fallback in domain classes | Avoid refactoring call sites | Silent failures in non-web contexts (API, cron) | Never — do not add new ones |
| `global $lang_id` / `global $settings` inside domain methods | No parameter threading | Testing impossible, context coupling | Never — do not add new ones |
| No DB transactions around multi-step order creation | Simpler code | Partial failures leave DB in inconsistent state | Never for financial flows |
| Price calculation duplicated in basket vs. admin recalculation | Independent paths, easy to patch individually | Divergence bugs when promotion/coupon logic changes | Never — needs single source of truth |
| `error_reporting(E_ALL ^ E_NOTICE ^ E_STRICT)` in ajax.php | Hides noise from legacy code | Masks real bugs (uninitialized variables) | Only during controlled migration |
| Raw SQL order number generation without lock | Simple | Duplicate order numbers under concurrency | Acceptable at very low order volume only |
---
## Integration Gotchas
| Integration | Common Mistake | Correct Approach |
|-------------|----------------|------------------|
| Przelewy24 webhook | Assuming single delivery — no idempotency guard | Check `$order['paid'] === 1` before processing, return 200 immediately |
| Tpay webhook | No status guard, will re-mark paid on replay | Add `if ($order['paid']) { echo 'TRUE'; exit; }` at top |
| Apilo payment sync | Calling sync before `apilo_order_id` is populated | Already handled by queue — do not bypass the queue mechanism |
| Apilo status sync | Hardcoded status ID `8` in `sendOrderToApilo()` regardless of actual order state | Should read from `ShopStatusRepository` like `syncApiloStatus()` does |
| Hotpay webhook | Recalculates summary from `price_netto` + VAT at callback time (line 100) — if product prices changed since order, hash verification will fail | Use stored `order.summary` instead of recalculating |
---
## Performance Traps
| Trap | Symptoms | Prevention | When It Breaks |
|------|----------|------------|----------------|
| `ProductRepository::findCached()` called in basket loop without Redis | N+1 DB queries per basket item on every page render | Ensure Redis is running; verify `CacheHandler` is not silently failing | ~50 concurrent sessions |
| `findPromotion()` called on every basket mutation | Full promotion table scan per AJAX call | Cache promotion query result in session with TTL | ~20 concurrent sessions |
| Dynamic `INFORMATION_SCHEMA` query in `sendOrderToApilo()` | Slow on busy DB servers | Replace with explicit column list | Production load |
| `get_new_version()` fetching remote URL on every admin page load (unless session cached) | Admin panel slow to load | Already has session cache — do not break the session cache path | Every admin page load without cache |
---
## Security Mistakes
| Mistake | Risk | Prevention |
|---------|------|------------|
| Tpay callback does not verify webhook signature | Fraudulent "paid" status injection — attacker sends `tr_status=TRUE&tr_crc=[valid_hash]` | Verify HMAC signature from Tpay before trusting `tr_status` |
| Hotpay signature uses hardcoded secret `"ProjectPro1916;"` in source code | Secret visible in source control; signature forgeable if secret leaks | Move secret to config file, never hardcode in PHP class |
| `Helpers::get()` reads both `$_POST` and `$_GET` without distinction | POST-only endpoints can be triggered via GET (bookmarkable, loggable in server logs) | In basket/order endpoints, validate request method explicitly |
| Admin ajax.php accessible without authentication check in the file itself | Relies on session set upstream; if session is misconfigured, admin actions exposed | Verify session auth at top of ajax.php as defense-in-depth |
---
## UX Pitfalls
| Pitfall | User Impact | Better Approach |
|---------|-------------|-----------------|
| Basket quantity decrease to 0 removes item silently (no undo) | User accidentally removes item with no recovery | This behavior is correct but edge cases in JS debounce can cause it — verify input validation on decrease |
| Payment confirmation page (`paymentConfirmation()`) fetches order by `order_hash` GET param — if user shares URL, anyone can see order details | Privacy breach for gift orders | Confirm access is restricted to logged-in client matching order, or token is sufficiently unguessable (md5 is weak) |
| Order number `generateOrderNumber()` resets to 001 each month — if run at midnight on month boundary, two orders can get the same number | Invoice numbering conflicts | See Pitfall 7 |
---
## "Looks Done But Isn't" Checklist
- [ ] **Coupon fix:** Coupon discount reflected in `pp_shop_orders.summary`, not just in session basket total — verify both match after fix.
- [ ] **Stock fix:** Stock decrements when order is created AND returns when order product is deleted from admin — verify `adjustStock()` is called symmetrically.
- [ ] **Payment webhook fix:** All three payment providers (Przelewy24, Tpay, Hotpay) have idempotency guards — do not fix only one.
- [ ] **Price calculation fix:** `calculateOrderSummaryByAdmin()` and `BasketCalculator::summaryPrice()` give the same result for the same input — write a manual test case.
- [ ] **Apilo sync fix:** Queue mechanism (`queueApiloSync`) is used by all payment paths — verify `cronJobRepo` is not null when called from `setOrderAsPaid`.
- [ ] **Session consistency:** After any basket mutation, session contains basket + recalculated promotions + valid transport cost — check all four controller methods, not just the one being fixed.
---
## Recovery Strategies
| Pitfall | Recovery Cost | Recovery Steps |
|---------|---------------|----------------|
| Duplicate coupon usage | LOW | Run `UPDATE pp_coupons SET used_count = (SELECT COUNT(*) FROM pp_shop_orders WHERE coupon_id = pp_coupons.id)` to resync |
| Negative stock | LOW | `UPDATE pp_shop_products SET quantity = 0 WHERE quantity < 0` — already partially handled in `createFromBasket()` |
| Order with wrong summary | MEDIUM | Admin can manually edit order products and transport in admin panel; summary recalculates |
| Duplicate order number | HIGH | Manual DB deduplication + notify customer; prevent recurrence with UNIQUE index migration |
| Lost order from `sendOrderToApilo` delete without transaction | HIGH | Restore from DB backup; add transaction wrapper immediately |
| Webhook replay double-payment | MEDIUM | Check payment provider dashboard for actual charge count; manually mark order status if double-charge did not occur |
---
## Pitfall-to-Phase Mapping
| Pitfall | Prevention Phase | Verification |
|---------|------------------|--------------|
| Price logic inconsistency across call sites | Phase 1 — basket/order bugfix | Manual: place order with coupon + promo; compare basket total, email total, DB `summary` |
| Double coupon usage | Phase 1 — order creation bugfix | Manual: submit order form twice quickly; check `used_count` in DB |
| Stock race condition | Phase 1 — order creation bugfix | Review: confirm atomic UPDATE or transaction wraps decrement |
| Payment webhook replay | Phase 1 — payment bugfix | Code review: idempotency guard present in all three handlers |
| Global `$mdb` fallback | Every phase — code review gate | Grep for `$GLOBALS['mdb']` in any new or modified method |
| Session state machine inconsistency | Phase 1 — basket bugfix | Manual: add product, apply coupon, remove product, check coupon still valid |
| Fragile order number generation | Phase 2 — stabilization | DB check: no duplicate `number` values in `pp_shop_orders` |
| `sendOrderToApilo` non-atomic clone/delete | Phase 2 — stabilization | Code review: transaction wraps all inserts and deletes |
| Tpay missing signature verification | Phase 1 — payment security | Code review: Tpay docs confirm correct HMAC field and secret |
| Hotpay hardcoded secret | Phase 1 — payment security | Config audit: secret moved to `config.php`, not in source class |
---
## Sources
- Direct codebase analysis: `autoload/Domain/Basket/BasketCalculator.php`
- Direct codebase analysis: `autoload/Domain/Order/OrderRepository.php` (createFromBasket, generateOrderNumber)
- Direct codebase analysis: `autoload/Domain/Order/OrderAdminService.php` (sendOrderToApilo, setOrderAsPaid)
- Direct codebase analysis: `autoload/front/Controllers/ShopOrderController.php` (paymentStatusTpay, paymentStatusPrzelewy24pl, paymentStatusHotpay)
- Direct codebase analysis: `autoload/front/Controllers/ShopBasketController.php` (basket mutation methods)
- Established patterns: PHP e-commerce concurrent stock decrement, webhook idempotency, coupon double-use (MEDIUM confidence from domain knowledge, corroborated by code structure)
---
*Pitfalls research for: custom PHP e-commerce shop bugfix/stabilization*
*Researched: 2026-03-10*

202
.planning/research/STACK.md Normal file
View File

@@ -0,0 +1,202 @@
# Stack Research
**Domain:** PHP e-commerce stabilization / bugfix (custom CMS, no framework)
**Researched:** 2026-03-10
**Confidence:** HIGH (all versions verified against Packagist and official sources)
---
## Project Context
The existing system is a frameworkless PHP shop (`autoload/` + `templates/`) using:
- Medoo ORM (direct SQL abstraction)
- RedBeanPHP ORM (co-existing with Medoo)
- Custom autoloader via `spl_autoload_register`
- No Composer (`composer.json` absent — dependencies bundled in `libraries/`)
- MySQL database with Redis cache
- Vanilla JS + SCSS frontend
This research covers **tools to add for stabilization**, not replacement of the existing runtime stack.
---
## Recommended Stack
### Core Testing Tools
| Technology | Version | Purpose | Why Recommended |
|------------|---------|---------|-----------------|
| PHPUnit | **11.x** | Unit and integration tests | PHPUnit 12+ requires PHP 8.3; PHPUnit 11 (requires PHP 8.2, supported until Feb 2026) is the safe bet for an unknown-PHP-version production system. Use 11 unless PHP version is confirmed >= 8.3. |
| Xdebug | **3.x** (latest 3.4+) | Code coverage + step debugging | Only PHP debugger with native coverage support that integrates with PHPUnit. Required for coverage reports. Use `xdebug.mode=coverage` for test runs, `xdebug.mode=off` in production. |
**PHPUnit version decision matrix:**
| Confirmed PHP version | Use PHPUnit |
|-----------------------|-------------|
| Unknown / < 8.2 | 10.x (requires PHP 8.1, EOL Feb 2025 — last resort) |
| 8.2 | **11.x** (supported until Feb 2026) |
| 8.3 | **12.x** (supported until Feb 2027) |
| 8.4+ | 13.x (latest stable 13.0.5) |
Confidence: HIGH — versions verified at https://phpunit.de/supported-versions.html and Packagist.
### Static Analysis
| Technology | Version | Purpose | Why Recommended |
|------------|---------|---------|-----------------|
| PHPStan | **2.1.40** | Type-safety and logic error detection | Dominant static analysis tool in PHP ecosystem (2025). PHPStan 2.x added Level 10 (strict mixed type analysis), 50-70% lower memory use. No framework plugin needed — works on plain PHP. Start at Level 1, raise gradually. |
Do NOT use Psalm for this project. Psalm has a smaller plugin ecosystem and slower analysis speed. PHPStan's standalone mode is a better fit for frameworkless PHP. Its taint analysis extension (`phpstan/extension-installer` + `phpstan-dba`) can catch SQL injection patterns in Medoo calls.
Confidence: HIGH — verified at https://github.com/phpstan/phpstan/releases/tag/2.0.0 and Packagist (2.1.40, released 2026-02-23).
### Code Style
| Technology | Version | Purpose | Why Recommended |
|------------|---------|---------|-----------------|
| PHP CS Fixer | **3.94.2** | Automatic code style enforcement | Auto-fixes (not just reports). PSR-12 ruleset is appropriate for this codebase. Reduces review friction by making style consistent before bugs are hunted. Run in `--dry-run` mode in CI, auto-fix locally. |
Do NOT use PHP_CodeSniffer for this project. PHPCS only detects violations — it does not auto-fix. PHP CS Fixer fixes them, which is more useful when stabilizing legacy code where style is inconsistent.
Confidence: HIGH — verified at Packagist (v3.94.2, released 2026-02-20).
### Automated Refactoring (Optional but Recommended)
| Technology | Version | Purpose | Why Recommended |
|------------|---------|---------|-----------------|
| Rector | **2.3.8** | Automated code upgrades and safe refactors | Rector can automatically modernize PHP patterns (e.g., `array()``[]`, missing return types, deprecated function calls) without manually touching every file. Run in dry-run first. Depends on PHPStan 2.1.38+. |
Confidence: HIGH — verified at Packagist (2.3.8, released 2026-02-22).
### Error Monitoring (Production)
| Technology | Version | Purpose | Why Recommended |
|------------|---------|---------|-----------------|
| Sentry PHP SDK | **4.21.0** | Production error capture and alerting | Catches uncaught exceptions and PHP errors in production with full stack traces, request context, and user impact metrics. Essential for a live shop — you need to know when real customers hit bugs. Free tier available; self-hosted option also free (Business plan features). |
Alternative: Bugsnag (`bugsnag/bugsnag`). Simpler setup but fewer features in free tier. Use Bugsnag if Sentry's free-tier limits are too low.
Do NOT rely on PHP's built-in `error_log` alone. It produces flat text files with no alerting, grouping, or frequency analysis. You will miss recurring bugs.
Confidence: MEDIUM — Sentry PHP SDK version verified on Packagist (4.21.0, 2026-02-24). Free tier limits may change.
### Database Debugging
| Technology | Version | Purpose | Why Recommended |
|------------|---------|---------|-----------------|
| MySQL Slow Query Log | Built-in | Identify slow/missing-index queries | The codebase already has `$database['long_query_time'] = 0.1` (100ms threshold) configured. Enable `slow_query_log` in MySQL config to start capturing queries that exceed this threshold. Zero-cost, no code changes required. |
Confidence: HIGH — native MySQL feature, configuration already exists in `config.php`.
---
## Supporting Libraries
| Library | Version | Purpose | When to Use |
|---------|---------|---------|-------------|
| `mockery/mockery` | ^1.6 | Test doubles for legacy classes | When PHPUnit's built-in mock builder is insufficient — particularly for classes with global state or static methods. |
| `phpstan/phpstan-strict-rules` | ^2.0 | Extra PHPStan strictness rules | Add after initial PHPStan baseline passes. Enforces stricter checks like strict comparisons. |
---
## Installation
No `composer.json` exists in this project. Composer must be initialized first.
```bash
# Initialize Composer (run once)
composer init --no-interaction
# Testing
composer require --dev phpunit/phpunit:^11.0
# Static analysis
composer require --dev phpstan/phpstan:^2.1
# Code style
composer require --dev friendsofphp/php-cs-fixer:^3.0
# Automated refactoring (optional)
composer require --dev rector/rector:^2.0
# Production error monitoring
composer require sentry/sentry:^4.0
```
---
## Alternatives Considered
| Recommended | Alternative | When to Use Alternative |
|-------------|-------------|-------------------------|
| PHPUnit 11 | Pest PHP | Pest has elegant syntax but is built on top of PHPUnit. For legacy frameworkless code, the added abstraction layer provides no benefit. Use PHPUnit directly. |
| PHPStan | Psalm | Only if you need Psalm's taint analysis for security audits. PHPStan is faster, has a larger plugin ecosystem, and is the industry standard in 2025. |
| PHP CS Fixer | PHP_CodeSniffer | Use PHPCS only if you need to report style violations without fixing them (e.g., in a formal code audit). |
| Sentry | Bugsnag | Bugsnag if team prefers simpler dashboards or if Sentry self-hosting is not viable. Both support PHP without frameworks. |
| MySQL Slow Query Log | `barryvdh/laravel-debugbar` style query logging | Laravel-specific tools won't work here. The MySQL-native slow log is the correct frameworkless approach. |
---
## What NOT to Use
| Avoid | Why | Use Instead |
|-------|-----|-------------|
| PHPUnit 12 or 13 | Requires PHP 8.3+. The production PHP version is unconfirmed. Jumping to 12/13 may break on the server. | PHPUnit 11 (PHP 8.2+) |
| Phan | Slower than PHPStan and Psalm, smaller community, harder to configure without Composer integration already in place. | PHPStan |
| PHP 7.x-era debug tools (Kint, Tracy standalone) | Useful for local dev only — they write HTML output to the page. Dangerous in production and irrelevant for systematic bugfix work. | Sentry for production; Xdebug for local debugging. |
| `var_dump` / `error_log` as sole debugging strategy | These do not persist, aggregate, or alert. They are acceptable for a single session but fail as a stabilization practice for a live shop. | Sentry SDK + MySQL slow query log. |
| RedBeanPHP alongside Medoo (current state) | The codebase uses both ORMs simultaneously (`rb.php` + `medoo.php`). This is a known source of connection state confusion. Neither should be added to — pick one for new code. | Medoo (already the primary ORM in Repository classes). |
---
## Stack Patterns by Variant
**If PHP version on server is >= 8.3:**
- Use PHPUnit 12.x (`^12.0`, supported until Feb 2027)
- PHPStan 2.x works at all PHP 8.x versions
**If PHP version on server is 8.2:**
- Use PHPUnit 11.x (`^11.0`, supported until Feb 2026)
- Plan upgrade to PHPUnit 12 before Feb 2026
**If no Composer on production server:**
- Use PHPUnit as a PHAR file for local/CI testing only
- Sentry SDK still requires Composer (use on production server separately)
**If Sentry cloud pricing is a concern:**
- Self-host Sentry (Docker-based, free, full Business plan features)
- Or use Bugsnag free tier (500 errors/day limit)
---
## Version Compatibility
| Package | Compatible With | Notes |
|---------|-----------------|-------|
| PHPUnit 11.x | PHP 8.2 8.3 | Safe current choice for unknown PHP version |
| PHPUnit 12.x | PHP 8.3+ | Use when PHP version is confirmed |
| PHPStan 2.x | PHP 7.4 8.4 (analysis target), PHP 8.1+ (to run PHPStan itself) | Can analyze PHP 7.4 code even when running on PHP 8.x |
| Rector 2.x | Requires PHPStan ^2.1.38 | Install PHPStan first |
| PHP CS Fixer 3.x | PHP 7.4+ | Works on any PHP 7.4+ codebase |
| Sentry SDK 4.x | PHP 7.2+ | No framework required |
| Xdebug 3.x | PHP 7.2 8.4 | Must match PHP version on server exactly — install via `pecl` or package manager |
---
## Sources
- https://phpunit.de/supported-versions.html — PHPUnit versions, PHP requirements, EOL dates (HIGH confidence)
- https://packagist.org/packages/phpunit/phpunit — PHPUnit 13.0.5 latest (2026-02-18) (HIGH confidence)
- https://packagist.org/packages/phpstan/phpstan — PHPStan 2.1.40 latest (2026-02-23) (HIGH confidence)
- https://packagist.org/packages/friendsofphp/php-cs-fixer — PHP CS Fixer v3.94.2 (2026-02-20) (HIGH confidence)
- https://packagist.org/packages/rector/rector — Rector 2.3.8 (2026-02-22) (HIGH confidence)
- https://packagist.org/packages/sentry/sentry — Sentry PHP SDK 4.21.0 (2026-02-24) (MEDIUM confidence)
- https://github.com/phpstan/phpstan/releases/tag/2.0.0 — PHPStan 2.0 release notes (HIGH confidence)
- https://phpunit.de/announcements/phpunit-12.html — PHPUnit 12 announcement (HIGH confidence)
- https://xdebug.org/docs/code_coverage — Xdebug 3 coverage configuration (HIGH confidence)
- https://dev.mysql.com/doc/refman/8.4/en/slow-query-log.html — MySQL slow query log (HIGH confidence)
---
*Stack research for: PHP e-commerce stabilization (pomysloweprezenty.pl)*
*Researched: 2026-03-10*

View File

@@ -0,0 +1,224 @@
# Project Research Summary
**Project:** pomysloweprezenty.pl — PHP e-commerce bugfix/stabilization
**Domain:** Custom frameworkless PHP e-commerce shop (production, live customers)
**Researched:** 2026-03-10
**Confidence:** HIGH (all four research areas based on direct codebase inspection + verified sources)
## Executive Summary
This is not a greenfield project — it is a live production e-commerce shop serving real customers in the Polish market. The system is a custom frameworkless PHP shop with no Composer, using Medoo and RedBeanPHP as co-existing ORMs, vanilla JS + SCSS on the frontend, and a session-based basket architecture. The stabilization milestone goal is to prevent revenue-impacting bugs, not to build new features. Research across all four areas points to the same conclusion: the highest-value work is auditing and hardening a small set of financial-critical code paths (order creation, price calculation, payment webhooks) rather than modernizing the architecture.
The recommended approach is targeted bugfixing organized around financial risk. Phase 1 must address the five critical paths where bugs directly cause revenue loss or trust damage: price calculation correctness across both the frontend checkout and admin edit paths, payment webhook idempotency across all three payment providers (Przelewy24, Tpay, Hotpay), stock decrement atomicity, coupon double-use prevention, and payment security gaps (missing Tpay HMAC signature verification, hardcoded Hotpay secret). Phase 2 consolidates operational reliability: Apilo sync verification, order number uniqueness mitigation, admin/frontend price path consistency, and email delivery guards. Phase 3 introduces tooling (PHPStan static analysis, PHP CS Fixer, PHPUnit) to prevent regressions going forward.
The primary risk is scope creep — the temptation to refactor while fixing. The codebase has identifiable technical debt (two ORMs, dual price calculation paths, global dependency fallbacks) but changing structural patterns during a stabilization phase will introduce new bugs. Every pitfall identified in research has a targeted fix that does not require architectural changes. The rule is: fix in the Domain layer, never in templates; inject dependencies, never add new global fallbacks; verify all call sites of any method touched.
---
## Key Findings
### Recommended Stack
No Composer exists in the project — it must be initialized before any dev tooling can be added. For testing, PHPUnit 11.x is the safe choice (PHP 8.2 requirement) unless the production PHP version is confirmed as 8.3+, in which case PHPUnit 12.x is preferred. PHPStan 2.x (currently 2.1.40) is the correct static analysis tool — it works on frameworkless PHP without plugins, supports Level 10 mixed type analysis, and has 5070% lower memory use than the 1.x series. PHP CS Fixer 3.x handles auto-fixing (not just reporting), which is the right choice for legacy codebases where style is inconsistent. Sentry PHP SDK 4.x provides production error capture — the current `error_log`-only approach produces flat files with no alerting or frequency analysis.
The MySQL slow query log is already partially configured in `config.php` (`$database['long_query_time'] = 0.1`) — enabling it in MySQL config requires zero code changes and will immediately surface N+1 query problems. Rector 2.x can automate PHP modernization (return types, array syntax) but should be introduced only after PHPStan passes.
**Core technologies:**
- PHPUnit 11.x: unit and integration tests — safe for unknown production PHP version (requires 8.2+)
- PHPStan 2.1.40: static analysis — no framework plugin needed, start at Level 1
- PHP CS Fixer 3.94.2: code style auto-fix — PSR-12 ruleset, reduces review friction on legacy code
- Sentry PHP SDK 4.21.0: production error monitoring — replaces flat `error_log` files with alerting
- MySQL slow query log (built-in): query performance — zero-cost, configuration already exists
- Rector 2.3.8 (optional): automated PHP modernization — run after PHPStan baseline passes
### Expected Features
This milestone is about what must work reliably, not what to build. The feature landscape maps directly to bug categories. All P1 fixes are revenue- or trust-critical (broken = customers leave or chargebacks occur). All P3 items are explicitly out of scope.
**Must fix — P1 (table stakes / revenue-critical):**
- Price calculation correctness — two independent calculation paths (BasketCalculator vs calculateOrderSummaryByAdmin) must produce identical results for the same input
- Hardcoded `payment_id == 3` magic number — cash-on-delivery auto-status-advance coupled to a DB record ID that can change silently
- Stock decrement atomicity — no transaction wrapping around order insert + stock decrement creates oversell risk
- Order number uniqueness — `MAX() + 1` without locking allows duplicate order numbers under concurrent submissions
- Payment webhook idempotency — all three providers (Przelewy24, Tpay, Hotpay) need idempotency guards; Tpay has no status guard at all
- Tpay missing signature verification and Hotpay hardcoded secret — active security vulnerabilities
- Order confirmation email URL correctness — `preg_replace` for relative-to-absolute URL rewriting has edge cases
**Must fix — P2 (operational reliability):**
- Apilo sync cron verification — queue introduced in recent commit only works if cron is actually running
- Admin/frontend price path consistency — `recalculateTransportCost()` and `BasketCalculator` must agree
- Status change email silent failure guard — missing template produces silent failure, not an error
- InPost/Orlen backend validation — frontend guard exists, backend re-check is absent
- Coupon object/array duality — `BasketCalculator` branches on `is_array($coupon)` with dual code paths
**Defer — P3:**
- Automated test suite from scratch — valuable long-term but delays actual fixes; add tests only for functions being fixed
- Session-to-DB basket migration — not a current pain point at this shop's scale
- New promotions or coupon types — explicitly out of scope per PROJECT.md
### Architecture Approach
The system has a clear layered architecture: four entry points (index.php, ajax.php, api.php, admin/ajax.php) dispatch through a routing layer into Domain objects organized by subdomain (Order, Basket, Product, Coupon, Promotion, Transport, etc.), backed by Medoo as the primary ORM. Newer code follows the Repository pattern with constructor injection. Older code uses legacy static factory classes with global state. Both generations coexist and must continue to coexist during bugfixing — migration is a separate project.
The bugfix protocol is: trace bugs to the Domain layer (Repository or Calculator), fix there, then verify all callers (frontend AJAX, API controllers, admin controllers, cron jobs may all call the same method). Never fix business logic in templates. Never add new code that falls back to `$GLOBALS['mdb']`.
**Major components:**
1. `Domain\Basket\BasketCalculator` — stateless price/stock calculation; the core of P1 pricing bugs; called from at least 3 entry points
2. `Domain\Order\OrderRepository` + `OrderAdminService` — order creation, status management, admin edit orchestration; the critical financial path
3. `Domain\Integrations` + `Domain\CronJob` — Apilo/ShopPro sync with DB-backed retry queue; recently refactored, needs operational verification
4. `front\controls\*` (legacy) + `front\Controllers\*` (DI) — two generations of frontend controllers coexisting; fix in the pattern of the code being touched
5. `Shared\Helpers\Helpers` (`\S::`) — global utility façade used everywhere; changing it has maximum blast radius, avoid during bugfix
**Safe change zones:** Domain Repositories, OrderAdminService, BasketCalculator (stateless, verifiable by input/output), API controllers, admin controllers.
**High blast radius zones:** `Shared\Helpers\Helpers`, `front\factory\*` static classes, `index.php` post-processing, `BasketCalculator::calculateBasketProductPrice()`.
### Critical Pitfalls
1. **Fixing price logic in one call site without mapping all callers**`BasketCalculator::summaryPrice()` is called from basket display, order creation, and admin recalculation. Fix one path without the others and totals diverge. Map all call sites with grep before touching any price logic.
2. **No DB transaction around order creation**`createFromBasket()` does sequential inserts/decrements with no `START TRANSACTION`. Partial failure leaves DB in inconsistent state (order exists, stock not decremented; or coupon used, order not created). Wrap financial flows in transactions.
3. **Payment webhook replay without idempotency** — Tpay has no status check before `setOrderAsPaid()`. All three payment providers need `if ($order['paid'] === 1) { exit; }` guard. Not adding this guard to all three providers when fixing one is a documented failure mode.
4. **Session state machine inconsistency** — Basket, coupon, and transport are stored as separate session keys. Not all basket mutation methods call the same recalculation steps. Read all four basket controller methods as a group before touching any of them.
5. **Global `$GLOBALS['mdb']` fallback in Domain classes** — Silently returns wrong data in API and cron contexts. Do not add new ones. When fixing a method that uses globals, pass the dependency explicitly — but only refactor the code path being touched, not all usages.
---
## Implications for Roadmap
Based on combined research, a 3-phase structure is recommended:
### Phase 1: Financial and Security Fixes
**Rationale:** All items here are revenue-impacting or active security vulnerabilities. Nothing in Phase 2 or 3 can be considered stable until these are addressed. These bugs affect real customers and real money today.
**Delivers:** A shop where prices are correct, payments are processed reliably, stock does not oversell, and payment provider integrations are not exploitable.
**Addresses (from FEATURES.md P1):**
- Price calculation correctness (both checkout and admin edit paths)
- Hardcoded `payment_id == 3` replaced with DB lookup or named constant
- Stock decrement wrapped in atomic UPDATE or transaction
- Payment webhook idempotency across all three providers (Przelewy24, Tpay, Hotpay)
- Tpay HMAC signature verification added
- Hotpay hardcoded secret moved to config
- Order confirmation email URL rewriting corrected
**Avoids (from PITFALLS.md):**
- Pitfall 1 (price logic missing call sites) — map all callers before touching BasketCalculator
- Pitfall 2 (double coupon usage) — transaction wrapping or idempotency guard on order creation
- Pitfall 3 (stock race condition) — atomic UPDATE or FOR UPDATE locking
- Pitfall 4 (webhook replay) — idempotency guards on all payment providers
- Pitfall 6 (session state machine) — read all basket methods as a group before fixing any
**Research flag:** No additional research needed. All bugs and fixes are identified from direct codebase analysis with high confidence.
---
### Phase 2: Operational Reliability
**Rationale:** These items do not directly cause immediate revenue loss per transaction but cause operational failures that accumulate (failed syncs, wrong admin totals, silent email failures). They depend on Phase 1 being stable first — fixing admin price recalculation while Phase 1 price logic is still broken creates confusion.
**Delivers:** An admin panel that reflects accurate order data, integrations that fail visibly rather than silently, and order number uniqueness mitigated against concurrency.
**Addresses (from FEATURES.md P2):**
- Apilo sync cron operational verification and admin visibility
- Admin/frontend transport cost consistency (`recalculateTransportCost` vs BasketCalculator)
- Status change email template existence guard (visible failure on missing template)
- InPost/Orlen backend pickup point validation
- Coupon object/array duality cleanup in BasketCalculator
**Avoids (from PITFALLS.md):**
- Pitfall 7 (order number generation) — apply `GET_LOCK()` as minimal mitigation; full UNIQUE index migration is a separate task
- Pitfall 8 (Apilo clone/delete non-atomic) — do not touch unless it is the direct cause of a reported bug; if touched, wrap in transaction
**Research flag:** Apilo API behavior may need verification when testing the retry queue drain. If Apilo rejects duplicate order numbers differently than expected, a quick API docs check is warranted.
---
### Phase 3: Tooling and Regression Prevention
**Rationale:** Introducing static analysis and code style tooling after the critical bugs are fixed provides the highest signal-to-noise ratio. Running PHPStan on unfixed code produces noise alongside real issues. After Phases 1 and 2, PHPStan will surface the remaining technical debt (global fallbacks, mixed types) cleanly.
**Delivers:** PHPStan baseline, PHP CS Fixer configuration, PHPUnit setup for regression tests on fixed code paths, Sentry production error monitoring.
**Uses (from STACK.md):**
- Composer initialization (prerequisite for all tooling)
- PHPUnit 11.x with targeted tests for BasketCalculator and OrderRepository
- PHPStan 2.x at Level 1 initially, raising gradually
- PHP CS Fixer 3.x with PSR-12 ruleset
- Sentry PHP SDK 4.x for production error capture
- MySQL slow query log enabled (zero-cost, already configured)
**Research flag:** PHP version on the production server must be confirmed before installing PHPUnit. If PHP 8.3+ is confirmed, use PHPUnit 12.x instead of 11.x.
---
### Phase Ordering Rationale
- Phase 1 before Phase 2: Financial bugs must not be present while operational reliability is being adjusted. Fixing admin price recalculation while the underlying BasketCalculator has a coupon bug means the admin "fix" inherits the same bug.
- Phase 2 before Phase 3: PHPStan run against code that still has known bugs produces a noisy baseline. A clean baseline after Phase 2 lets PHPStan focus on structural issues, not known functional bugs.
- Tooling in Phase 3, not Phase 1: Writing tests for unfixed code encodes the wrong behavior. Tests should be written immediately after fixing each function in Phase 1, as part of that fix, not as a separate earlier initiative.
- No anti-features introduced: All three phases explicitly exclude basket-to-DB migration, new coupon/promo types, and JS framework modernization — all confirmed out of scope by FEATURES.md analysis.
### Research Flags
Phases likely needing deeper research during planning:
- **Phase 1 (Tpay HMAC):** Tpay webhook signature verification requires consulting current Tpay API documentation to confirm the correct HMAC field name and signing algorithm — the current code does not implement it at all, so the correct implementation must be looked up.
- **Phase 3 (Production PHP version):** The PHPUnit version decision (11.x vs 12.x) is blocked on confirming the PHP version running on the production server. Check `phpinfo()` or server config before installing.
Phases with standard patterns (can proceed without additional research):
- **Phase 1 (stock race condition):** Atomic `UPDATE ... SET quantity = GREATEST(0, quantity - ?)` is a standard MySQL pattern — no research needed.
- **Phase 1 (payment idempotency):** Early-return guard pattern (`if ($order['paid']) exit`) is well-established — no research needed.
- **Phase 2 (PHPStan baseline):** PHPStan 2.x baseline generation is well-documented — standard invocation with `--generate-baseline`.
- **Phase 3 (Composer init):** Standard `composer init` workflow — no research needed.
---
## Confidence Assessment
| Area | Confidence | Notes |
|------|------------|-------|
| Stack | HIGH | All package versions verified on Packagist with exact release dates. Version selection rationale is solid. One caveat: production PHP version unconfirmed — affects PHPUnit version choice only. |
| Features | HIGH | Derived entirely from direct codebase analysis. Bug locations are identified at specific line numbers. Prioritization reflects actual code risk, not general e-commerce patterns. |
| Architecture | HIGH | Based on direct inspection of all entry points, Domain layer, routing layer, and template system. Component boundaries and data flows are documented from code, not inference. |
| Pitfalls | HIGH | Eight critical pitfalls identified with exact file and line references. Recovery strategies provided for each. Integration gotchas verified against actual webhook handler code. |
**Overall confidence:** HIGH
### Gaps to Address
- **Production PHP version:** Must be confirmed before Phase 3 tooling installation. SSH into the server and run `php -v`, or add `phpinfo()` to a temporary test page. This determines PHPUnit 11.x vs 12.x.
- **Tpay HMAC algorithm:** The correct field name and HMAC signing method for Tpay webhook signature verification must be looked up in current Tpay API documentation before implementing the fix in Phase 1.
- **Cron execution confirmation:** The CronJob queue introduced in a recent commit (`cron.php`) needs operational verification — confirm it is registered in the server's crontab and that jobs drain within expected TTL. This is a Phase 2 verification task, not a code task.
- **Free delivery threshold consistency:** Two code paths compute transport cost (`BasketCalculator` at checkout, `recalculateTransportCost()` in admin edit). A side-by-side comparison of their logic with the same inputs should be done at the start of Phase 2 to establish whether they diverge and under what conditions.
---
## Sources
### Primary (HIGH confidence)
- Direct codebase analysis: `autoload/Domain/Basket/BasketCalculator.php` — pricing, stock, coupon logic
- Direct codebase analysis: `autoload/Domain/Order/OrderRepository.php` — order creation, number generation, payment handling
- Direct codebase analysis: `autoload/Domain/Order/OrderAdminService.php` — admin edit, Apilo sync, status change
- Direct codebase analysis: `autoload/front/Controllers/ShopOrderController.php` — Tpay, Przelewy24, Hotpay webhook handlers
- Direct codebase analysis: `autoload/front/Controllers/ShopBasketController.php` — basket mutation methods
- https://phpunit.de/supported-versions.html — PHPUnit version support and PHP requirements
- https://packagist.org/packages/phpstan/phpstan — PHPStan 2.1.40 (2026-02-23)
- https://packagist.org/packages/friendsofphp/php-cs-fixer — PHP CS Fixer v3.94.2 (2026-02-20)
- https://packagist.org/packages/rector/rector — Rector 2.3.8 (2026-02-22)
- https://packagist.org/packages/sentry/sentry — Sentry PHP SDK 4.21.0 (2026-02-24)
- https://dev.mysql.com/doc/refman/8.4/en/slow-query-log.html — MySQL slow query log
### Secondary (MEDIUM confidence)
- General PHP e-commerce patterns for webhook idempotency and concurrent stock decrement — corroborated by code structure
- Sentry free tier limits — verified on Packagist but pricing may change; self-hosted option available
### Tertiary (LOW confidence)
- Tpay HMAC signing algorithm — not verified against current Tpay API documentation; must be confirmed before Phase 1 payment fix implementation
---
*Research completed: 2026-03-10*
*Ready for roadmap: yes*