From b1b2cc58272a7d14386ce6659e338a64a9f10ecb Mon Sep 17 00:00:00 2001 From: Jacek Pyziak Date: Sun, 26 Apr 2026 21:39:12 +0200 Subject: [PATCH] chore: generate codebase map in .paul/codebase/ INDEX, STACK, ARCHITECTURE, CONVENTIONS, TESTING, INTEGRATIONS, CONCERNS --- .paul/codebase/ARCHITECTURE.md | 506 +++++------------------ .paul/codebase/CONCERNS.md | 311 ++++----------- .paul/codebase/CONVENTIONS.md | 710 +++++++-------------------------- .paul/codebase/INDEX.md | 89 ++--- .paul/codebase/INTEGRATIONS.md | 116 ++++++ .paul/codebase/STACK.md | 323 +++------------ .paul/codebase/TESTING.md | 114 ++++++ 7 files changed, 625 insertions(+), 1544 deletions(-) create mode 100644 .paul/codebase/INTEGRATIONS.md create mode 100644 .paul/codebase/TESTING.md diff --git a/.paul/codebase/ARCHITECTURE.md b/.paul/codebase/ARCHITECTURE.md index 4db9bc8..4c46e39 100644 --- a/.paul/codebase/ARCHITECTURE.md +++ b/.paul/codebase/ARCHITECTURE.md @@ -1,434 +1,126 @@ # Architecture -**Analysis Date:** 2026-03-12 - ---- - -## 1. Directory Structure +## Request Flow ``` -orderPRO/ # Project root -├── bin/ # CLI scripts (cron, migrations, data tools) -├── bootstrap/ -│ └── app.php # Application bootstrap: autoload, env, config, DI wiring -├── config/ -│ ├── app.php # Application config (name, debug, locale, paths, cron, secrets) -│ └── database.php # DB connection params -├── database/ -│ ├── migrations/ # SQL migration files (47 files, named YYYYMMDD_NNNNNN_description.sql) -│ └── drafts/ # Non-active schema drafts -├── DOCS/ -│ ├── ARCHITECTURE.md # Existing architecture notes (Polish, incremental) -│ ├── DB_SCHEMA.md # Table-level schema reference -│ ├── TECH_CHANGELOG.md # Chronological technical changelog -│ └── todo.md # Work backlog -├── public/ -│ ├── index.php # Single HTTP entry point -│ └── assets/ # Compiled CSS, JS, images (served directly) -├── resources/ -│ ├── lang/ # Translation files (PL locale) -│ ├── modules/ # Frontend JS/CSS modules (e.g. jquery-alerts) -│ ├── scss/ # SCSS source → compiled to public/assets/css/ -│ └── views/ # PHP template files (plain PHP, no engine) -├── routes/ -│ └── web.php # All route definitions (returns closure) -├── src/ -│ ├── Core/ # Framework kernel (router, DB, HTTP, security, i18n, views) -│ └── Modules/ # Feature modules (Auth, Orders, Settings, Cron, Shipments, Users) -├── storage/ -│ ├── cache/ # File cache -│ ├── logs/ # app.log -│ ├── sessions/ # PHP session files -│ └── tmp/ # Temp files (e.g. downloaded labels) -└── tests/ - └── Unit/ # Unit tests (currently empty) -``` - ---- - -## 2. Application Layers - -### Layer 1 — Kernel (`src/Core/`) - -Framework infrastructure. No business logic. - -| Class | File | Responsibility | -|---|---|---| -| `Application` | `src/Core/Application.php` | Root service locator. Wires all dependencies, boots session, registers error handlers, runs cron on web. | -| `Router` | `src/Core/Routing/Router.php` | Pattern-matching router for GET/POST. Supports `{param}` segments and middleware pipeline. | -| `Request` | `src/Core/Http/Request.php` | Immutable wrapper around `$_GET`, `$_POST`, `$_FILES`, `$_SERVER`. | -| `Response` | `src/Core/Http/Response.php` | Fluent response builder (`html()`, `json()`, `redirect()`). | -| `Template` | `src/Core/View/Template.php` | Output-buffer PHP view renderer. Injects `$e()` (XSS escape) and `$t()` (translation) helpers. Supports layout wrapping. | -| `ConnectionFactory` | `src/Core/Database/ConnectionFactory.php` | Creates PDO connection from config array. | -| `Migrator` | `src/Core/Database/Migrator.php` | Applies pending `.sql` files from `database/migrations/`. Uses DB lock (`GET_LOCK`) to prevent concurrent runs. | -| `Csrf` | `src/Core/Security/Csrf.php` | Session-based CSRF token generate/validate using `hash_equals`. | -| `Session` | `src/Core/Support/Session.php` | Thin wrapper around PHP sessions. | -| `Flash` | `src/Core/Support/Flash.php` | Single-request flash messages stored in `$_SESSION`. | -| `Logger` | `src/Core/Support/Logger.php` | Appends `[LEVEL] message {context_json}` lines to `storage/logs/app.log`. | -| `Env` | `src/Core/Support/Env.php` | Parses `.env` file; `get()`, `bool()` helpers. | -| `Translator` | `src/Core/I18n/Translator.php` | Loads language files from `resources/lang/`; `get($key, $replace)`. | - -### Layer 2 — Modules (`src/Modules/`) - -Feature slices. Each module owns its controllers, repositories and services. - -| Module | Path | Active Modules | -|---|---|---| -| Auth | `src/Modules/Auth/` | Login/logout, session-based auth | -| Orders | `src/Modules/Orders/` | Order list/detail, status change, import | -| Settings | `src/Modules/Settings/` | Statuses, integrations (Allegro/Apaczka/InPost/shopPRO), DB, cron, company | -| Cron | `src/Modules/Cron/` | Job scheduling, execution, handlers per job type | -| Shipments | `src/Modules/Shipments/` | Provider-agnostic shipment creation, label download | -| Users | `src/Modules/Users/` | User management (list, create) | - -### Layer 3 — Views (`resources/views/`) - -Plain PHP templates. No Blade, no Twig. - -| Path | Renders for | -|---|---| -| `layouts/app.php` | Main authenticated shell (sidebar nav) | -| `layouts/auth.php` | Login page shell | -| `orders/list.php` | Order list page | -| `orders/show.php` | Order detail page | -| `settings/allegro.php` | Allegro integration settings | -| `settings/apaczka.php` | Apaczka integration settings | -| `settings/inpost.php` | InPost integration settings | -| `settings/shoppro.php` | shopPRO integration settings (multi-instance) | -| `settings/statuses.php` | Status groups & statuses management | -| `settings/cron.php` | Cron scheduler UI | -| `settings/integrations.php` | Integrations hub | -| `settings/database.php` | Migrations UI | -| `settings/company.php` | Company/sender settings | -| `shipments/prepare.php` | Shipment creation form | -| `users/index.php` | Users list | -| `auth/login.php` | Login form | -| `components/table-list.php` | Reusable paginated table component | -| `components/order-status-panel.php` | Reusable order status panel | - ---- - -## 3. Bootstrap and Request Flow - -### Bootstrap sequence - -``` -public/index.php - └─ bootstrap/app.php - ├─ autoload (vendor/autoload.php or spl_autoload from src/) - ├─ Env::load('.env') - ├─ load config/app.php + config/database.php - ├─ new Application($basePath, $config) - │ ├─ new Router - │ ├─ new Translator - │ ├─ new Template - │ ├─ ConnectionFactory::make → PDO - │ ├─ new UserRepository, OrdersRepository, OrderStatusRepository - │ ├─ new Migrator - │ └─ new AuthService, Logger - └─ $app->boot() - ├─ prepareDirectories (storage/*) - ├─ configureSession - ├─ registerErrorHandlers - └─ require routes/web.php → $routes($app) - (instantiates all controllers, repositories, services and registers routes) -``` - -### Per-request flow - -``` -HTTP request +HTTP Request → public/index.php - → $app->run() - ├─ Request::capture() (wraps $_GET/$_POST/$_FILES/$_SERVER) - ├─ maybeRunCronOnWeb() (if cron_run_on_web=1, throttled, DB-locked) - └─ Router::dispatch($request) - ├─ find route by method + path (exact match first, then {param} patterns) - ├─ attach URL params to $request via withAttributes() - ├─ build middleware pipeline (array_reduce, reversed) - │ └─ AuthMiddleware::handle() (most routes) - │ └─ redirect /login if not authenticated - └─ call controller method($request): Response - ├─ reads input via $request->input() - ├─ validates CSRF via Csrf::validate() - ├─ calls repository/service methods - └─ returns Response::html($template->render(...)) - or Response::redirect(...) - or Response::json(...) - → Response::send() (sets headers, echoes body) + → bootstrap/app.php (loads config, registers PDO, services) + → Application::boot() (loads routes/web.php) + → Router::dispatch(Request) (matches URL, runs middleware pipeline) + → [Middleware] (AuthMiddleware, ApiKeyMiddleware) + → Controller::method() (parse input → call repository/service → render) + → Template::render() (PHP native, layout composition) + → Response::send() ``` ---- +## Layer Map -## 4. Module Organization +| Layer | Location | Responsibility | +|-------|----------|----------------| +| Entry | `public/index.php` | Bootstrap only | +| Routes | `routes/web.php` (581 lines) | All ~80 routes; manual DI wiring | +| Core | `src/Core/` (25 files) | Framework infrastructure | +| Controllers | `src/Modules/*/Controller.php` | Request parsing → response | +| Services | `src/Modules/*/Service.php` | Business logic | +| Repositories | `src/Modules/*/Repository.php` | PDO data access (34+ repos) | +| Views | `resources/views/` | PHP templates with `$e()` / `$t()` | +| Components | `resources/views/components/` | Reusable UI blocks | -### Auth (`src/Modules/Auth/`) +## Module Inventory (`src/Modules/`) -| Class | Responsibility | -|---|---| -| `AuthController` | `GET /login`, `POST /login`, `POST /logout` | -| `AuthService` | `check()`, `user()`, `attempt(email, password)`, `logout()` backed by session | -| `AuthMiddleware` | `handle(Request, callable $next)` — redirects to `/login` if session unauthenticated | +| Module | Files | Key Classes | Purpose | +|--------|-------|-------------|---------| +| **Auth** | 3 | `AuthController`, `AuthMiddleware`, `AuthService` | Login/logout, session | +| **Users** | 2 | `UserController`, `UserRepository` | User CRUD | +| **Orders** | 3 | `OrdersController` (1187 LOC), `OrdersRepository` (1221 LOC) | Order list, detail, status, payment, correlated subquery for return-risk | +| **Shipments** | 17 | `ShipmentController`, provider services + tracking services | Shipment creation, label download, tracking polling | +| **Accounting** | 5 | `AccountingController`, `ReceiptService`, `ReceiptRepository` | Receipts, invoices, PDF, Excel export | +| **Email** | 3 | `EmailSendingService`, `VariableResolver`, `AttachmentGenerator` | Template-based email with PDF attachments | +| **Automation** | 6 | `AutomationService` (834 LOC), `AutomationRepository`, `AutomationExecutionLogRepository` | Event→condition→action rules, email triggers | +| **Settings** | 51+ | Integration controllers, OAuth clients, API clients, mappers | Allegro/shopPRO/Apaczka/InPost config, status mappings | +| **Cron** | 12 | `CronRepository`, `CronHandlerFactory`, handler classes | Scheduled imports, syncs, token refresh | +| **Printing** | 4 | `PrintApiController`, `PrintJobRepository`, `ApiKeyMiddleware` | REST API for Windows print client | +| **Statistics** | 2 | `OrdersStatisticsController`, `OrdersStatisticsRepository` | Dashboard aggregates | +| **Info** | 1 | `InfoController` | Health check | -### Orders (`src/Modules/Orders/`) +## Key Data Flows -| Class | Responsibility | -|---|---| -| `OrdersController` | `index` (list), `show` (detail), `updateStatus` — builds view data, delegates DB to repos | -| `OrdersRepository` | `paginate()`, `findDetails()`, `statusCounts()`, `statusPanelConfig()`, `sourceOptions()`, `updateOrderStatus()`, `recordActivity()` | -| `OrderImportRepository` | `upsertOrderAggregate()` — transactional upsert of the full order aggregate (order + addresses + items + payments + shipments + notes + status history) | +### Order Lifecycle +1. **Import** — Cron handler → API client → `OrderImportService` → `OrdersRepository::insertOrder()` → `AutomationService::executeForNewOrder()` +2. **Status update** — `OrdersController::updateStatus()` → `OrdersRepository::updateStatus()` → automation check +3. **Status sync** — Cron → `AllegroStatusSyncService` / `ShopproStatusSyncService` → carrier API -### Settings (`src/Modules/Settings/`) +### Shipment Flow +1. **Create** — `ShipmentController::create()` → `ShipmentProviderRegistry` → carrier `ShipmentService::createShipment()` → `ShipmentPackageRepository::insert()` +2. **Track** — Cron `ShipmentTrackingHandler` → `ShipmentTrackingRegistry` → carrier tracking API → `ShipmentPackageRepository::updateDeliveryStatus()` -Large module covering all configuration: +### Receipt / Invoice +1. **Generate** — `ReceiptController::store()` → `ReceiptService::generateReceipt()` → `ReceiptRepository::insert()` + Dompdf PDF +2. **Email** — `EmailSendingService::send()` → `VariableResolver::resolve()` → `AttachmentGenerator::generatePdf()` → PHPMailer SMTP -**Controllers:** -- `SettingsController` — statuses, status groups, DB migrations -- `AllegroIntegrationController` — Allegro OAuth2, import, status mappings, delivery mappings -- `ApaczkaIntegrationController` — Apaczka API config -- `InpostIntegrationController` — InPost ShipX config -- `ShopproIntegrationsController` — shopPRO multi-instance management (statuses, delivery, sync) -- `IntegrationsHubController` — overview hub listing all provider instances -- `CronSettingsController` — cron schedule UI -- `CompanySettingsController` — company/sender details +### Automation Rules +1. **Setup** — `AutomationController` → `AutomationRepository::insertRule()` +2. **Trigger** — `AutomationService::executeForOrder()` → evaluates trigger (`order_status_changed`, `order_status_aged`) → runs action (send email, update status) +3. **Log** — `AutomationExecutionLogRepository` tracks every run -**Repositories:** -- `OrderStatusRepository` — CRUD + reorder for `order_status_groups` and `order_statuses` -- `AllegroIntegrationRepository` — reads/writes `allegro_integration_settings`; encrypts secrets via `IntegrationSecretCipher` -- `AllegroStatusMappingRepository` — `allegro_order_status_mappings` CRUD -- `AllegroOrderSyncStateRepository` — cursor state for Allegro sync (`integration_order_sync_state`) -- `ApaczkaIntegrationRepository` — reads/writes `apaczka_integration_settings` -- `InpostIntegrationRepository` — reads/writes `inpost_integration_settings` -- `IntegrationsRepository` — base `integrations` table queries (hub listing) -- `ShopproIntegrationsRepository` — multi-instance CRUD for `integrations` where `type=shoppro` -- `ShopproStatusMappingRepository` — `order_status_mappings` per shopPRO integration -- `ShopproDeliveryMethodMappingRepository` — `shoppro_delivery_method_mappings` -- `CarrierDeliveryMethodMappingRepository` — `carrier_delivery_method_mappings` (unified, cross-source) -- `CompanySettingsRepository` — `company_settings` single-row config -- `AllegroDeliveryMethodMappingRepository` — Allegro-specific delivery method mappings (legacy, superseded by carrier table) +### Cron Jobs -**API Clients (HTTP adapters):** -- `AllegroOAuthClient` — builds OAuth2 authorize URL, exchanges code for tokens, refreshes tokens -- `AllegroApiClient` — calls Allegro REST API (`/order/checkout-forms`, `/sale/product-offers`, `/delivery-services`, etc.) -- `ApaczkaApiClient` — calls Apaczka REST API (services, create shipment, label, points) -- `ShopproApiClient` — calls shopPRO REST API (orders, products, dictionaries, statuses) +| Handler | Task | +|---------|------| +| `AllegroOrdersImportHandler` | Fetch new Allegro orders | +| `AllegroStatusSyncHandler` | Push status changes to Allegro | +| `AllegroTokenRefreshHandler` | OAuth token refresh (24h expiry) | +| `ShopproOrdersImportHandler` | Fetch new shopPRO orders | +| `ShopproStatusSyncHandler` | Push status to shopPRO | +| `ShopproPaymentStatusSyncHandler` | Sync payment statuses | +| `ShipmentTrackingHandler` | Poll carrier tracking APIs | +| `OrderStatusAgedHandler` | Trigger automation for stuck statuses | +| `AutomationHistoryCleanupHandler` | Purge old automation logs | -**Services:** -- `AllegroOrderImportService` — maps single Allegro checkout-form to neutral order aggregate, calls `OrderImportRepository::upsertOrderAggregate()` -- `AllegroOrdersSyncService` — paginates Allegro checkout-forms list and imports new/changed orders; maintains sync cursor -- `AllegroStatusSyncService` — syncs statuses between Allegro and orderPRO based on configured direction -- `AllegroStatusDiscoveryService` — fetches live status list from Allegro API and seeds `allegro_order_status_mappings` -- `ShopproOrdersSyncService` — fetches shopPRO orders, maps to neutral aggregate, handles paczkomat/pickup points, media -- `ShopproStatusSyncService` — orchestrates shopPRO→orderPRO status synchronisation -- `ShopproPaymentStatusSyncService` — polls shopPRO `paid` flag and updates `orders.payment_status` -- `IntegrationSecretCipher` — AES-256-CBC encryption with HMAC-SHA256 authentication (`v1:base64(iv+mac+cipher)`) +## Dependency Injection -### Cron (`src/Modules/Cron/`) - -| Class | Responsibility | -|---|---| -| `CronRunner` | Dispatches due schedules to `cron_jobs` queue; processes pending jobs up to `$limit`; delegates to typed handlers | -| `CronRepository` | `findDueSchedules()`, `claimNextPendingJob()`, `markJobCompleted()`, `markJobFailed()`, `enqueueJobFromSchedule()`, `getBoolSetting()`, `getIntSetting()` | -| `AllegroTokenRefreshHandler` | `handle()` → refreshes Allegro OAuth token before expiry | -| `AllegroOrdersImportHandler` | `handle()` → calls `AllegroOrdersSyncService::sync()` | -| `AllegroStatusSyncHandler` | `handle()` → calls `AllegroStatusSyncService::sync()` | -| `ShopproOrdersImportHandler` | `handle()` → calls `ShopproOrdersSyncService::syncAll()` | -| `ShopproStatusSyncHandler` | `handle()` → calls `ShopproStatusSyncService::syncAll()` | -| `ShopproPaymentStatusSyncHandler` | `handle()` → calls `ShopproPaymentStatusSyncService::syncAll()` | - -Cron is also triggerable via CLI (`bin/cron.php`) and optionally runs on each HTTP request when `cron_run_on_web=1` (throttled with session timer + DB advisory lock `orderpro_web_cron_lock`). - -### Shipments (`src/Modules/Shipments/`) - -Provider-agnostic shipment layer built around `ShipmentProviderInterface`: +Manual constructor injection in `routes/web.php` — no DI container library. Example: ```php -interface ShipmentProviderInterface { - public function code(): string; - public function getDeliveryServices(): array; - public function createShipment(int $orderId, array $formData): array; - public function checkCreationStatus(int $packageId): array; - public function downloadLabel(int $packageId, string $storagePath): array; -} +$ordersController = new OrdersController( + $template, $translator, $auth, + $app->orders(), $shipmentPackageRepository, + $receiptRepository, $receiptConfigRepository, ... +); ``` -| Class | Provider Code | Responsibility | -|---|---|---| -| `AllegroShipmentService` | `allegro_wza` | Allegro WZA carrier integration | -| `ApaczkaShipmentService` | `apaczka` | Apaczka carrier API: wycena, create, label, points fallback for pickup addresses | -| `ShipmentProviderRegistry` | — | Map of `code → ShipmentProviderInterface`; resolved by `ShipmentController` | -| `ShipmentController` | — | `prepare` (form), `create`, `checkStatus`, `label` endpoints | -| `ShipmentPackageRepository` | — | CRUD for `shipment_packages` table | +All production classes are `final` — prevents accidental inheritance. -### Users (`src/Modules/Users/`) - -| Class | Responsibility | -|---|---| -| `UsersController` | List users, create user | -| `UserRepository` | Find by email, list, insert/update | - ---- - -## 5. Core Domain Concepts - -### Order Aggregate - -The order domain is provider-neutral. All external orders (Allegro, shopPRO) are normalised into the same tables: +## Directory Structure ``` -orders ← main record (source, integration_id, statuses, totals, flags) - ├─ order_addresses ← 1:N (type: customer | delivery | invoice) - ├─ order_items ← 1:N (products, quantities, media_url) - ├─ order_payments ← 1:N (payment method, amounts) - ├─ order_shipments ← 1:N (tracking numbers, provider info) - ├─ order_documents ← 1:N (invoices, etc.) - ├─ order_notes ← 1:N (internal notes) - ├─ order_status_history ← 1:N (status transitions with timestamps) - └─ order_activity_log ← 1:N (universal event log: status_change, payment, shipment, import, note, ...) +bootstrap/ app.php (service wiring, config loading) +bin/ migrate.php, cron.php (CLI entry points) +config/ app.php, database.php +database/ + migrations/ 84 SQL files (YYYYMMDD_NNNNNN_description.sql) + drafts/ WIP migrations +public/ + index.php HTTP entry point + .htaccess Apache rewrite rules + assets/css/ Compiled CSS (app.css, login.css, modules/) + assets/js/ jquery-alerts.js, global-search.js, automation-form.js +resources/ + views/ PHP templates by module + components/ layouts/ + scss/ SCSS sources (app.scss, login.scss, modules/_*.scss) + modules/ jquery-alerts JS+SCSS source + lang/pl/ Polish translations +routes/ + web.php All routes (581 lines) +src/ + Core/ Framework (25 files) + Modules/ 13 feature modules (~200+ PHP files) +storage/ + logs/ app.log + sessions/ PHP session files + cache/ PHPUnit cache, etc. +tests/ + Unit/ PHPUnit tests (7+ service test files) + bootstrap.php PSR-4 autoloader for tests ``` - -Key `orders` columns: -- `source` — origin system (`allegro`, `shoppro`) -- `source_order_id` — external ID in source system -- `integration_id` — FK to `integrations.id` -- `internal_order_number` — `OPXXXXXXXXX` (unique internal identifier, e.g. `OP000000001`) -- `external_status_id` / `external_status_code` — last known status from source -- `payment_status` — payment state (`0`=unpaid, `2`=paid, etc.) -- `ordered_at` / `source_created_at` / `source_updated_at` / `fetched_at` — date fallback chain - -### Integration Registry (`integrations` table) - -Single base table for all integration instances. Specific settings are in satellite tables linked by `integration_id`: - -``` -integrations (type, name, is_active, api_key_encrypted, orders_fetch_enabled, ...) - ├─ allegro_integration_settings (1:1, environment, OAuth tokens, token_expires_at) - ├─ apaczka_integration_settings (1:1, app_id, app_secret_encrypted) - └─ inpost_integration_settings (1:1, api_token, organization_id, defaults) - -integrations (type=shoppro, multi-instance) - ├─ order_status_mappings (1:N, shoppro_status_code → orderpro_status_code) - └─ shoppro_delivery_method_mappings (1:N, delivery method → carrier service) -``` - -### Status Configuration - -``` -order_status_groups (name, code, color_hex, sort_order, is_active) - └─ order_statuses (1:N, name, code, sort_order, is_active) -``` - -Status codes in `orders` (`external_status_code`) are mapped to human labels via join with `order_statuses`. Fallback group "Pozostale" catches unmapped codes. - -External status mappings per provider: -- `allegro_order_status_mappings` — `allegro_status_code → orderpro_status_code` -- `order_status_mappings` — generic per-integration mapping (`shoppro_status_code → orderpro_status_code`) - -### Carrier / Delivery Method Mappings - -``` -carrier_delivery_method_mappings - source_system + source_integration_id + order_delivery_method - → provider + provider_service_id + provider_account_id -``` - -This is the unified resolution table used by `ShipmentController` to auto-select the carrier service when preparing a shipment. - -### Cron Scheduling Tables - -``` -cron_schedules (job_type, interval_seconds, priority, is_active, next_run_at) - → cron_jobs (job_type, payload, status, priority, scheduled_at, result_json, ...) -``` - -`CronRunner` reads due schedules, enqueues jobs, then claims and processes pending jobs up to the configured limit. - ---- - -## 6. Dependency Injection Pattern - -The application does not use a DI container. All dependencies are wired manually in `routes/web.php` (for HTTP) and in `Application::maybeRunCronOnWeb()` (for web cron). This means: - -- Every controller receives its dependencies via constructor injection. -- `Application` holds the core singletons (PDO, AuthService, OrdersRepository, etc.) and exposes them via typed getters (`$app->db()`, `$app->orders()`, `$app->auth()`, etc.). -- `routes/web.php` is the composition root for the web context. - ---- - -## 7. Security Architecture - -| Concern | Mechanism | -|---|---| -| Authentication | Session-based. `AuthService::check()` reads `$_SESSION`. `AuthMiddleware` guards all protected routes. | -| CSRF | `Csrf::token()` / `Csrf::validate()` — single session token, validated on all POST handlers before any mutation. | -| Secret storage | API keys and OAuth tokens encrypted at-rest using `IntegrationSecretCipher` (AES-256-CBC + HMAC-SHA256). Secret key comes from `INTEGRATIONS_SECRET` env var. | -| XSS | Template helper `$e()` wraps `htmlspecialchars()` with `ENT_QUOTES`. | -| SQL injection | All queries use PDO prepared statements (medoo-style named params). No raw string concatenation. | -| DB locks | `GET_LOCK` used for migrations (`orderpro_migrations_lock`) and web cron (`orderpro_web_cron_lock`) to prevent concurrent execution. | - ---- - -## 8. Database Design Summary - -**Migration system:** SQL files in `database/migrations/` named `YYYYMMDD_NNNNNN_description.sql`, sorted alphabetically and tracked in `migrations` table. Applied via `Migrator::runPending()` from UI or CLI (`bin/migrate.php`). - -**Core tables:** - -| Table | Purpose | -|---|---| -| `users` | Panel users (email, password_hash) | -| `orders` | Master order record | -| `order_addresses` | Typed addresses per order (customer/delivery/invoice) | -| `order_items` | Order line items with media_url | -| `order_payments` | Payment records per order | -| `order_shipments` | Shipment/tracking records from source system | -| `order_documents` | Document references (invoices etc.) | -| `order_notes` | Internal notes | -| `order_status_history` | Full status transition history | -| `order_activity_log` | Universal event log (status_change, payment, shipment, import, note, document, message) | -| `order_status_groups` | Configurable status groups (with color, sort) | -| `order_statuses` | Statuses within groups | -| `order_status_mappings` | External status → orderPRO status per integration (generic) | -| `allegro_order_status_mappings` | Allegro-specific status mappings | -| `integrations` | Base table for all integration instances (allegro, apaczka, inpost, shoppro) | -| `allegro_integration_settings` | Allegro OAuth2 credentials and tokens (1:1 with integrations) | -| `apaczka_integration_settings` | Apaczka app_id + secrets (1:1 with integrations) | -| `inpost_integration_settings` | InPost ShipX config and defaults (1:1 with integrations) | -| `integration_order_sync_state` | Import cursor per integration (last synced order, timestamps) | -| `carrier_delivery_method_mappings` | Unified: order delivery method → shipment provider service | -| `shoppro_delivery_method_mappings` | shopPRO-specific delivery mappings (legacy, superseded by carrier table) | -| `shipment_packages` | Packages created via shipment providers (status, tracking, label path) | -| `company_settings` | Single-row sender company config (address, contact_person, package defaults) | -| `cron_schedules` | Named job schedules with intervals and priorities | -| `cron_jobs` | Job queue: pending, running, completed, failed | -| `app_settings` | Key-value settings store (cron_run_on_web, allegro_status_sync_direction, etc.) | -| `migrations` | Applied migration filenames (Migrator tracking) | - ---- - -## 9. CLI Scripts (`bin/`) - -| Script | Purpose | -|---|---| -| `bin/cron.php` | CLI cron runner; loads app and calls `CronRunner::run($limit)` | -| `bin/migrate.php` | CLI migration runner; calls `Migrator::runPending()` | -| `bin/deploy_and_seed_orders.php` | Applies order schema draft + seeds test data (`--count`, `--append`, `--profile=default\|realistic`) | -| `bin/fix_status_codes.php` | Repairs status group/status codes (PL→ASCII transliteration, `--dry-run`) | -| `bin/fill_order_item_images.php` | Backfills `order_items.media_url` from integration APIs | -| `bin/randomize_order_statuses.php` | Dev utility to randomize order statuses | -| `bin/fix_gs1_brand.php` | Fixes GS1 brand data | -| `bin/test_gs1_api.php` | Tests GS1 API connectivity | - ---- - -## 10. Frontend Architecture - -- **Styles:** SCSS source in `resources/scss/`, compiled to `public/assets/css/`. -- **Scripts:** JS in `resources/modules/` (e.g. `jquery-alerts`), compiled to `public/assets/js/modules/`. -- **Alerts/confirms:** `window.OrderProAlerts.confirm(...)` from `public/assets/js/modules/jquery-alerts.js`. Native `alert()`/`confirm()` are forbidden. -- **Views:** Plain PHP; no JS framework. Inline JS kept minimal, page-level only in view files. -- **Layout:** Single shell `resources/views/layouts/app.php` with sidebar nav. Sidebar `activeMenu` and `activeSettings` variables control highlighted state. - ---- - -*Full architecture analysis: 2026-03-12* diff --git a/.paul/codebase/CONCERNS.md b/.paul/codebase/CONCERNS.md index cae3525..5e60c2f 100644 --- a/.paul/codebase/CONCERNS.md +++ b/.paul/codebase/CONCERNS.md @@ -1,264 +1,93 @@ -# Codebase Concerns +# Technical Concerns & Debt -**Analysis Date:** 2026-03-12 +## God Classes (Priority Refactor Targets) ---- +| Class | LOC | Methods | Issue | +|-------|-----|---------|-------| +| `src/Modules/Orders/OrdersRepository.php` | 1,221 | 29 | Query building spread across 29 methods; SonarQube S1448 | +| `src/Modules/Orders/OrdersController.php` | 1,187 | 22 | UI + AJAX + list + detail + search combined; S1448 | +| `src/Modules/Automation/AutomationService.php` | 834 | 24 | All action handlers in one class; S1448 | +| `src/Modules/Settings/ShopproOrderMapper.php` | 867 | 25 | 25+ transformation methods; S1448 | +| `src/Modules/Settings/ApaczkaShipmentService.php` | 1,044 | — | API payload deeply nested | +| `src/Modules/Settings/ShopproIntegrationsController.php` | 1,044 | — | OAuth + mapping + sync combined | -## Tech Debt +**Planned fix:** Phase 68 (Code Deduplication Refactor) — deferred, never started. +## SonarQube Issues (new code since 2026-03-28) -### [MEDIUM] SonarQube — 327 Open Issues (2026-03-12) +**Total: 174 issues** (BLOCKER=1, CRITICAL=47, MAJOR=110, MINOR=16) -Listed in `DOCS/todo.md`: -- 95x `php:S112` — generic `new \Exception` instead of typed exceptions -- 57x `php:S1142` — excessive `return` statements per method -- 40x `php:S1192` — repeated string literals not extracted to constants -- 31x `php:S3776` — high cognitive complexity -- 11x `php:S1448` — classes with too many methods (god classes) -- 4x `php:S138` — methods over the allowed line limit +| Rule | Count | Severity | Examples | +|------|-------|----------|---------| +| `php:S112` — Generic exceptions | 95+ | MAJOR | `throw new \Exception(...)` everywhere; use domain exceptions | +| `php:S1142` — Excess return statements | 57+ | MAJOR | Many service methods have 4-10 returns | +| `php:S1192` — Duplicated string literals | 39 | CRITICAL | Route paths, status strings, HTTP headers | +| `php:S3776` — Cognitive complexity > 15 | 9+ | CRITICAL | `ShopproOrderMapper::initOrderFromArray()` (28), `ShipmentTrackingHandler` (27) | +| `php:S1448` — Class too large | 6+ | MAJOR | See god classes above | +| `php:S1172` — Unused parameters | 11+ | MAJOR | `$request` params in handlers, unused payload params | +| `php:S4423` — Weak TLS protocol | 1 | **CRITICAL** | `EmailMailboxController::testConnection()` line ~223: `fsockopen('ssl://...')` — deprecated | +| `php:S5911` — Missing import | 1 | **BLOCKER** | `AllegroOrderImportService` — `RuntimeException` not imported | +| `php:S4833` — Use namespace import | 2 | MAJOR | `resources/views/accounting/index.php:31`, `orders/show.php:780` | +| `Web:S6827` — Anchors without accessible text | 9+ | MINOR | Icon-only buttons need `aria-label` | +| `Web:S6819` — Accessibility | 7+ | MAJOR | Use `` instead of `` | -The most critical from a maintainability standpoint are the complexity and god-class violations. +**Note:** SonarQube scan not run for phases 105–107 — baseline may be stale. ---- +## Known Bugs & Issues -### [LOW] Duplicate Migration Number `000014` +| Issue | Location | Status | +|-------|----------|--------| +| `STAT-NET`: hardcoded 23% VAT fallback for net calculations | `src/Modules/Statistics/OrdersStatisticsRepository.php:471` | Deferred (`.paul/TODO.md`) | +| Missing net amounts for shopPRO orders | `.paul/TODO.md` (STAT-NET) | Deferred | +| `order.status_aged` condition fallback | `AutomationService` | Fixed 2026-04-25 | -- Issue: Two migration files share sequence number `000014`: - - `20260227_000014_create_product_integration_translations.sql` - - `20260301_000014_add_products_sku_format_setting.sql` -- File: `database/migrations/` -- Impact: Depending on how the migrator detects "already run" (by filename vs. sequence number), one of these may silently never execute or may fail. -- Fix approach: Rename one to `000014b` or renumber all subsequent ones. Verify migrator uses full filename as key. +## Deferred Indexes (Phase 106) ---- +After Phase 106 (customer return alert), two indexes should be added before dataset exceeds ~50k orders: -## Security Concerns +```sql +-- INDEX-106-01 (deferred in SUMMARY.md) +CREATE INDEX idx_order_addresses_order_type ON order_addresses(order_id, address_type); +CREATE INDEX idx_shipment_packages_order_delivery ON shipment_packages(order_id, delivery_status); +``` -### [HIGH] No SSL Verification on cURL Calls +These support the correlated subquery in `OrdersRepository` used for return-risk detection. -- Issue: None of the `AllegroApiClient`, `ShopproApiClient`, `ApaczkaApiClient`, or `AllegroOAuthClient` set `CURLOPT_SSL_VERIFYPEER` or `CURLOPT_SSL_VERIFYHOST`. PHP's default is to verify SSL, but explicitly not setting it means behavior depends on the server's `php.ini` `curl.cainfo` configuration. -- Files: - - `src/Modules/Settings/AllegroApiClient.php` - - `src/Modules/Settings/AllegroOAuthClient.php` - - `src/Modules/Settings/ShopproApiClient.php` - - `src/Modules/Settings/ApaczkaApiClient.php` -- Impact: On environments without a properly configured CA bundle (common on shared hosting / Windows XAMPP), all API calls may proceed without SSL verification, enabling MITM attacks on OAuth tokens and API keys. -- Fix approach: Explicitly set `CURLOPT_SSL_VERIFYPEER => true` and configure `CURLOPT_CAINFO` pointing to a known CA bundle. Document this in `.env.example`. +## Security Items to Verify ---- +| Item | Risk | Action | +|------|------|--------| +| `print_api_keys.api_key` encryption | MEDIUM | Verify column is encrypted (same as `integrations.api_key_encrypted`) | +| `fsockopen('ssl://...')` in `EmailMailboxController::testConnection()` | MEDIUM | Replace with `stream_socket_client()` + `stream_context_create(['ssl' => ['verify_peer' => true]])` | +| Email variable injection via `{{var}}` templates | LOW | Only predefined variables allowed — verify automation rule creation doesn't accept arbitrary variable names | -### [MEDIUM] CSRF Token Is Never Rotated After Login +## Architecture Concerns -- Issue: `Csrf::token()` in `src/Core/Security/Csrf.php` generates the token once per session and never regenerates it. The token is never rotated after a successful login, and it is never invalidated after a POST form is submitted. -- File: `src/Core/Security/Csrf.php` -- Impact: If a CSRF token leaks (e.g., via Referer header, browser history), it remains valid for the entire session lifetime. -- Fix approach: Rotate the CSRF token on login. Optionally regenerate after each use (per-form tokens). At minimum, invalidate the session token on `AuthService::logout()`. +| Concern | Impact | Notes | +|---------|--------|-------| +| No repository interfaces | LOW | Cannot mock repositories cleanly in tests without `bypass-finals` workaround | +| String-typed event/action names | LOW | `event_type = 'order.status_changed'` — typos not caught at compile time | +| No validation layer | LOW | Validation scattered across controllers and repositories | +| No HTTP caching headers | LOW | Responses don't set ETag, Cache-Control; acceptable for low-concurrency use | +| No query caching | LOW | Every request re-queries; no Redis/Memcached layer | ---- +## Duplication Areas (Phase 68 scope) -### [MEDIUM] Label File Served with `file_get_contents($fullPath)` — Partial Path Traversal Risk +- `SslCertificateResolver` — pattern duplicated across multiple API client files +- `ToggleableRepositoryTrait` — mixes query building + toggling +- `RedirectPathResolver` — similar redirect logic in 4+ controllers +- `ReceiptService` vs accounting logic — overlapping responsibilities -- Issue: In `ShipmentController::label()`, the file path is built from `$package['label_path']` retrieved from the database. While `basename()` is used for the filename in the response header, `$fullPath` itself is derived by concatenating `$storagePath . '/labels/' . $filename` from the shipment service. There is no `realpath()` check confirming the file is actually within the storage directory before serving it. -- File: `src/Modules/Shipments/ShipmentController.php` (lines 292–316) -- Impact: Low risk currently (path comes from DB, not user input directly), but if `label_path` column is ever injectable or incorrectly set, arbitrary file reads become possible. -- Fix approach: Validate `realpath($fullPath)` starts with `realpath($storagePath)` before calling `file_get_contents()`. +## Legacy / Deprecated Patterns ---- +| Pattern | Location | Status | +|---------|----------|--------| +| `fsockopen('ssl://')` | `EmailMailboxController::testConnection()` | Deprecated PHP TLS approach — fix when touching that method | +| `require` in views | `resources/views/accounting/index.php:31` | Should use namespace `use` — minor | +| Raw `$_SESSION` access | Some older controllers | Should use `Session::get()` / `set()` helpers | -### [LOW] Allegro OAuth Callback Endpoint Has No Auth Middleware +## Performance Risks -- Issue: `GET /settings/integrations/allegro/oauth/callback` is registered without `$authMiddleware`. The OAuth state parameter is validated via session, which does provide CSRF protection for the OAuth flow, but any authenticated check is absent. -- File: `routes/web.php` (line 239) -- Impact: An unauthenticated actor with knowledge of the state parameter could complete an OAuth token exchange. In practice, the state must match the session, limiting exploitability to session-fixation scenarios. -- Fix approach: Add `$authMiddleware` to the callback route. Handle the "not authenticated yet" edge case by storing the session state before redirect. - ---- - -## Performance Concerns - -### [HIGH] N+1 Correlated Subqueries on Every Order List Row - -- Issue: The `OrdersRepository::paginate()` list SQL includes three correlated subqueries per row: `items_count`, `items_qty`, `shipments_count`, and `documents_count`. These fire once per order in the result set. -- File: `src/Modules/Orders/OrdersRepository.php` (lines 124–127) -- Impact: For a page of 50 orders: 4 × 50 = 200 additional subquery executions per page load. Degrades significantly as the `orders` table grows. -- Fix approach: Replace correlated subqueries with LEFT JOIN + GROUP BY aggregates, or preload counts in a single separate query (`SELECT order_id, COUNT(*) ... GROUP BY order_id WHERE order_id IN (...)`). - ---- - -### [HIGH] `canResolveMappedMedia()` Queries `information_schema.COLUMNS` on Every Request - -- Issue: `OrdersRepository::canResolveMappedMedia()` runs a query against `information_schema.COLUMNS` to check whether optional product mapping tables exist. It caches the result in a private property, but this property is instance-scoped — the check fires once per HTTP request that instantiates `OrdersRepository`. -- File: `src/Modules/Orders/OrdersRepository.php` (lines 604–648) -- Impact: `information_schema` queries are notoriously slow on MySQL. This fires on every order-related page load (list, detail, shipment). -- Fix approach: Cache the result in a static class property or in the app-level cache/session so it survives across requests. Alternatively, remove the check entirely — if the schema is managed by migrations, the tables either exist or don't. - ---- - -### [MEDIUM] AllegroStatusSyncService Fetches Up to 50 Orders Then Re-Imports Each One Fully - -- Issue: `AllegroStatusSyncService::findOrdersNeedingStatusSync()` returns up to 50 Allegro orders. For each, `importSingleOrder()` is called, which makes a full Allegro API call per order (checkout-form fetch + optional shipments fetch + optional offer image fetch per line item). This is 50+ API calls per cron run. -- File: `src/Modules/Settings/AllegroStatusSyncService.php` -- Impact: Rate-limit exhaustion on Allegro API. Slow cron runs. -- Fix approach: Use a lighter-weight API call to fetch only status fields for multiple orders (e.g., `listCheckoutForms` with a filter) rather than a full re-import per order. -- Note (2026-03-13): Plan 02-02 dodał kursor `last_status_checked_at` — eliminuje re-import zamówień bez zmiany statusu. Full API call per order pozostaje osobnym concernem. - ---- - -### [MEDIUM] No Database Indexes Verified for Key Queries - -- Issue: Queries frequently filter and sort on `orders.source`, `orders.external_status_id`, `orders.ordered_at`, `order_items.order_id`, and `allegro_order_status_mappings.allegro_status_code`. Whether indexes exist for these columns is not documented. -- Files: `src/Modules/Orders/OrdersRepository.php`, `database/migrations/20260302_000018_create_orders_tables_and_schedule.sql` -- Impact: Full table scans as data grows. -- Fix approach: Audit the migration that creates `orders` and related tables for index definitions. Add missing indexes for `(source, external_status_id)`, `(ordered_at)`, and `allegro_status_code`. - ---- - -## Architectural Concerns - -### [HIGH] `Settings` Module Is a God Module — Contains Unrelated Concerns - -- Issue: The `src/Modules/Settings/` directory contains 30+ classes covering: Allegro integration, Apaczka integration, InPost integration, shopPRO integration, company settings, cron settings, order statuses, carrier mappings, delivery method mappings, and OAuth. These are logically distinct domains crammed into a single module namespace. -- Files: All files in `src/Modules/Settings/` -- Impact: The module boundary is meaningless. New integrations will continue to pile into Settings. `AllegroIntegrationController` (923 lines) and `ShopproIntegrationsController` (901 lines) are god classes by SonarQube rules (S1448). -- Fix approach: Gradually decompose into purpose-built modules: `src/Modules/Integrations/Allegro/`, `src/Modules/Integrations/Shoppro/`, `src/Modules/Integrations/Apaczka/`, `src/Modules/Integrations/Inpost/`. - ---- - -### [HIGH] `AllegroIntegrationController` and `ShopproIntegrationsController` Are God Classes (900+ lines) - -- Issue: Each controller handles: displaying settings, saving credentials, OAuth flow, status mapping CRUD, delivery method mapping CRUD, and import triggering. Each is 900+ lines. -- Files: - - `src/Modules/Settings/AllegroIntegrationController.php` (923 lines) - - `src/Modules/Settings/ShopproIntegrationsController.php` (901 lines) -- Impact: Violates SRP. Methods average far more than 50 lines. Cognitive complexity is high (SonarQube S3776, 31 violations). -- Fix approach: Split each into focused controllers per tab/concern: e.g., `AllegroCredentialsController`, `AllegroStatusMappingController`, `AllegroDeliveryMappingController`. - ---- - -### [HIGH] `ShopproOrdersSyncService` Is 1192 Lines — Largest Single File - -- Issue: `ShopproOrdersSyncService` handles the full import lifecycle for shopPRO orders including: pagination, cursor management, order mapping, address building, item building, image resolution, payment mapping, shipment mapping, status history, and activity logging — all in one class. -- File: `src/Modules/Settings/ShopproOrdersSyncService.php` -- Impact: Highest complexity class in the codebase. Hard to test, hard to modify, prone to regression. -- Fix approach: Extract a `ShopproOrderMapper` (mapping logic), a `ShopproImageResolver` (image fetch), and keep `ShopproOrdersSyncService` as the orchestrator. Mirror the pattern of `AllegroOrderImportService` which is better separated. - ---- - -### [MEDIUM] No Dependency Injection Container — Full Object Graph in `routes/web.php` - -- Issue: `routes/web.php` manually constructs all services and controllers, including multiple separate instantiations of identical objects (e.g., `new AllegroApiClient()` appears 3 times, `new OrdersRepository($app->db())` appears 4 times). -- File: `routes/web.php` -- Impact: Multiple object instances where singletons are implied. Adding new dependencies requires editing this file, which is already 257 lines. Risk of silently passing the wrong instance. -- Fix approach: Introduce a lightweight DI container or service locator. At minimum, hoist shared instances (`$allegroApiClient`, `$ordersRepository`) to named variables and reuse them. - ---- - - -## Incomplete Features - -### [HIGH] 5 Stub Buttons with No Actions in Order Detail View - -- Issue: The order detail page (`resources/views/orders/show.php`, lines 48–53) contains 5 buttons that render in the UI but have no `href`, no form action, and no JavaScript handler: "Strefa klienta", "Platnosc", "Drukuj", "Pakuj", "Edytuj". -- File: `resources/views/orders/show.php` -- Impact: Users see clickable but non-functional UI elements. The "Pakuj" (pack) button is styled as the primary CTA (`btn--primary`). -- Fix approach: Either implement the features or hide the buttons until they are ready. - ---- - -### [HIGH] `orderpro_to_allegro` Status Sync Direction Not Implemented - -- Issue: The setting UI offers two sync directions: Allegro → orderPRO and orderPRO → Allegro. The second direction explicitly returns early with "Kierunek orderPRO -> Allegro nie jest jeszcze wdrozony" ("not yet implemented"). -- File: `src/Modules/Settings/AllegroStatusSyncService.php` (lines 38–45) -- Impact: Users who configure the "push status to Allegro" direction get silent no-op behavior with no clear user-facing indication in the cron log. -- Fix approach: Either implement the feature or disable the `orderpro_to_allegro` direction option in the UI until it is ready. - ---- - -### [HIGH] TODO in `DOCS/todo.md` — UI Issues Open - -The following items from `DOCS/todo.md` are marked incomplete: -- Item 12: Status change sync with Allegro after manual user change -- Item 14: Input/select/textarea border color needs to be darker -- Item 15: Source and ID display order should be reversed (source first, then ID) -- Item 16: Order list statuses should be colored according to settings -- Item 17: Source display should show specific integration name instead of "shopPRO"; ID label missing -- Files: `resources/views/orders/list.php`, `resources/views/orders/show.php` - ---- - -### [MEDIUM] InPost Integration — Settings Page Exists But No Shipment Provider - -- Issue: `InpostIntegrationController` and `InpostIntegrationRepository` exist and the settings page at `/settings/integrations/inpost` is functional. However, there is no `InpostShipmentService` implementing `ShipmentProviderInterface`. InPost labels are currently routed through `allegro_wza` as a workaround (`provider_code === 'inpost'` is remapped to `'allegro_wza'` in `ShipmentController::create()`). -- Files: - - `src/Modules/Settings/InpostIntegrationController.php` - - `src/Modules/Settings/InpostIntegrationRepository.php` - - `src/Modules/Shipments/ShipmentController.php` (line 165–166) -- Impact: InPost credentials are stored but not used by their own provider. The remap to `allegro_wza` means InPost shipments are actually created via Allegro WZA. This breaks if the user has only InPost credentials and no Allegro integration. -- Fix approach: Implement `InpostShipmentService implements ShipmentProviderInterface` using the stored InPost API token. - ---- - -### [MEDIUM] `database/drafts/` Contains Uncommitted Schema SQL - -- Issue: `database/drafts/20260302_orders_schema_v1.sql` exists in the `drafts` subdirectory. This is not a migration — it will not be executed by the migrator. -- File: `database/drafts/20260302_orders_schema_v1.sql` -- Impact: Unclear whether this represents schema that has not yet been migrated or schema that was migrated by other means. Creates confusion about source of truth. -- Fix approach: Either move to `database/migrations/` with a proper sequence number, or delete it if already superseded. - ---- - -## Known Bugs - - -## Test Coverage Gaps - -### [HIGH] No Integration or Functional Tests Exist - -- Issue: The `tests/` directory contains only `bootstrap.php`. No test files exist. The `archive/` directory contains unit tests from a previous reset, but they are not in active use. -- Files: `tests/bootstrap.php`, `archive/2026-03-02_users-only-reset/tests/` -- Risk: Zero automated coverage for any critical path: order import, shipment creation, OAuth flow, cron sync, status updates. -- Priority: High - ---- - -### [HIGH] Allegro OAuth Token Refresh Logic Has No Tests - -- Issue: The token refresh logic is centralized in `AllegroTokenManager` but untested. It includes complex edge cases: token expiry within 5-minute window, empty refresh token fallback, write-then-re-read pattern. -- Files: `src/Modules/Settings/AllegroTokenManager.php` -- Risk: Token refresh failures cause complete import failure. Silent breakage possible. -- Priority: High - ---- - -### [MEDIUM] No Error Path Tests for Order Import - -- Issue: `AllegroOrderImportService::importSingleOrder()` and `ShopproOrdersSyncService::sync()` have complex error handling (401 retry, silent Throwable catch blocks). None of this is tested. -- Files: - - `src/Modules/Settings/AllegroOrderImportService.php` - - `src/Modules/Settings/ShopproOrdersSyncService.php` -- Risk: Error paths that swallow exceptions silently (multiple `catch (Throwable) {}` blocks in `OrdersRepository`) may hide data integrity issues. -- Priority: Medium - ---- - -## Fragile Areas - -### [HIGH] `AllegroStatusSyncService` — `orderpro_to_allegro` Silent No-Op - -- Files: `src/Modules/Settings/AllegroStatusSyncService.php` -- Why fragile: Returns `ok: true` when direction is `orderpro_to_allegro` with zero processing, no log, no user feedback. Cron logs will show "success" even though nothing happened. -- Safe modification: Do not add code after the early return without understanding why it was left as a no-op. Consider returning `ok: false` or a warning until the feature is implemented. -- Test coverage: None. - ---- - -### [MEDIUM] Web Cron Throttling Uses Session Timestamp (`$_SESSION['cron_web_last_run_at']`) - -- Issue: `Application::isWebCronThrottled()` reads and writes a cron timestamp from `$_SESSION`. In stateless or load-balanced deployments, different users hit different sessions, meaning the throttle is per-session (per-user), not global. -- File: `src/Core/Application.php` (lines 363–375) -- Why fragile: With two active sessions, the cron may run twice within the throttle window. The DB-level `GET_LOCK` is the real protection, but this creates confusing interactions. -- Fix approach: Move last-run timestamp to the database (e.g., an `app_settings` key) instead of the session. - ---- - -*Concerns audit: 2026-03-12* +1. **Correlated subquery for return-risk** (per-row COUNT) — slow at >50k orders without `INDEX-106-01` +2. **N+1 potential in order details** — `findDetails()` queries orders + addresses + items separately (not verified as actual problem) +3. **Cron queue growth** — no exponential backoff if queue grows; may pile up on slow syncs diff --git a/.paul/codebase/CONVENTIONS.md b/.paul/codebase/CONVENTIONS.md index 40ae074..010876d 100644 --- a/.paul/codebase/CONVENTIONS.md +++ b/.paul/codebase/CONVENTIONS.md @@ -1,603 +1,183 @@ -# Coding Conventions +# Conventions & Patterns -**Analysis Date:** 2026-03-12 - ---- - -## 1. Documented Rules (AGENTS.md / CLAUDE.md) - -These rules are canonical and enforced across the project: - -- Code must be readable by a stranger — clear names, no magic -- No logic in views, no copy-paste, no random helpers without coherent design -- Each function/class has **one responsibility**, typically 30–50 lines; split if longer -- Max **3 levels of nesting** (if/foreach); move deeper logic to separate methods -- Comments only where they explain **why**, never **what** -- DB: Medoo is mentioned in docs but the codebase uses **raw PDO** with prepared statements — no string-concatenated SQL for user input -- XSS: escape all output in views with the `e()` helper -- CSRF token required on all POST forms, validated via `Csrf::validate()` -- All styles live in `resources/scss/`, never inline in view files -- Never use native `alert()` / `confirm()` — use `window.OrderProAlerts.confirm()` -- Reusable UI blocks go in `resources/views/components/` - ---- - -## 2. Naming Conventions - -### PHP +## Naming | Element | Convention | Example | -|---|---|---| -| Classes | PascalCase | `OrdersRepository`, `ShipmentProviderRegistry` | -| Methods | camelCase | `findDetails()`, `updateOrderStatus()` | -| Variables | camelCase | `$orderId`, `$statusLabelMap` | -| Constants | UPPER_SNAKE_CASE | `SESSION_KEY` (in `Csrf`) | -| Namespaces | PascalCase, PSR-4 | `App\Modules\Orders`, `App\Core\Http` | -| Interfaces | PascalCase + `Interface` suffix | `ShipmentProviderInterface` | -| Test classes | PascalCase + `Test` suffix | `CronJobTypeTest` | +|---------|-----------|---------| +| Classes | PascalCase | `OrdersController`, `AllegroApiClient` | +| Methods / variables | camelCase | `findDetails()`, `$statusCode` | +| Constants | UPPER_SNAKE_CASE | `SESSION_KEY`, `OPTION_KEYS` | +| DB columns | snake_case | `source_order_id`, `payment_status` | +| PHP files | Match class name | `OrdersController.php` | +| View files | kebab-case | `table-list.php`, `order-status-panel.php` | +| SCSS partials | `_kebab-case.scss` | `_automation.scss` | +| No abbreviations | Full names | `$translatedText` not `$t` (except loop indices) | -No abbreviations in names. `$orderId` not `$oid`. Loop variables in short 2–3 line loops are the only exception. +## Code Constraints (CLAUDE.md) -### Files +- Max **~50 lines** per method; longer → split +- Max **3 nesting levels** (if/foreach); deeper → extract to method +- Single Responsibility: one class = one job +- All classes are `final` (no accidental inheritance) +- `declare(strict_types=1)` in every file +- Comments only for **WHY**, never for WHAT -- One class per file, filename matches class name exactly -- `PascalCase.php` for all PHP class files -- `kebab-case` or `snake_case` not used in PHP +## Database Pattern -### CSS / SCSS - -- BEM-style class naming: `block__element--modifier` -- Examples: `.sidebar__brand`, `.orders-ref__meta`, `.btn--primary`, `.is-active` -- State modifier classes: `.is-active`, `.is-collapsed`, `.is-open`, `.is-disabled` -- JavaScript hook classes: `.js-` prefix (e.g. `.js-sidebar-collapse`, `.js-filter-toggle-btn`) - ---- - -## 3. PHP Code Style - -### Strict Types - -Every PHP file starts with: -```php ->, total:int, page:int, per_page:int, error:string} - */ -public function paginate(array $filters): array -``` - -### Defensive Casting - -All values from external sources (request input, DB rows) are cast explicitly before use: -```php -$orderId = max(0, (int) $request->input('id', 0)); -$search = trim((string) ($filters['search'] ?? '')); -$totalPaid = $row['total_paid'] !== null ? (float) $row['total_paid'] : null; -``` - ---- - -## 4. Design Patterns - -### Service Layer Pattern - -Business logic lives in dedicated `*Service` classes, not controllers or repositories: -- `src/Modules/Settings/AllegroOrderImportService.php` — order import logic -- `src/Modules/Settings/ShopproStatusSyncService.php` — status sync logic -- `src/Modules/Settings/ShopproPaymentStatusSyncService.php` - -Controllers delegate to services, which delegate to repositories. - -### Repository Pattern - -All database access goes through `*Repository` classes. No direct PDO calls in controllers or services beyond what repositories expose: -- `src/Modules/Orders/OrdersRepository.php` -- `src/Modules/Settings/AllegroIntegrationRepository.php` -- `src/Modules/Shipments/ShipmentPackageRepository.php` - -### Interface + Registry Pattern - -Provider integrations use an interface + registry: -```php -// src/Modules/Shipments/ShipmentProviderInterface.php -interface ShipmentProviderInterface -{ - public function code(): string; - public function createShipment(int $orderId, array $formData): array; - // ... -} - -// src/Modules/Shipments/ShipmentProviderRegistry.php -final class ShipmentProviderRegistry -{ - public function get(string $code): ?ShipmentProviderInterface { ... } - public function all(): array { ... } -} -``` -Registered at boot time in `routes/web.php`: -```php -$shipmentProviderRegistry = new ShipmentProviderRegistry([ - $shipmentService, // AllegroShipmentService - $apaczkaShipmentService, -]); -``` - -### Cron Handler Pattern - -Cron jobs follow a handler contract. Each job type has a dedicated handler class with a `handle(array $payload): array` method: -- `src/Modules/Cron/AllegroOrdersImportHandler.php` -- `src/Modules/Cron/ShopproStatusSyncHandler.php` -- `src/Modules/Cron/AllegroTokenRefreshHandler.php` - -Handlers are registered as a string-keyed array in `CronRunner`: -```php -$runner = new CronRunner($repository, $logger, [ - 'allegro_orders_import' => new AllegroOrdersImportHandler($ordersSyncService), - 'shoppro_order_status_sync' => new ShopproStatusSyncHandler($shopproStatusSyncService), -]); -``` - -### Manual Dependency Injection (No Container) - -There is no DI container. All dependencies are wired manually in `routes/web.php` and `Application.php`. The Application class acts as a **Service Locator** for top-level objects (`$app->db()`, `$app->orders()`, `$app->logger()`), but controllers and services receive dependencies through constructor injection. - -### Middleware Pipeline - -Middleware uses a `handle(Request $request, callable $next): Response` or `__invoke` signature. The router builds a pipeline with `array_reduce`: -```php -// src/Core/Routing/Router.php -$pipeline = array_reduce( - array_reverse($middlewares), - fn (callable $next, callable|object $middleware): callable => $this->wrapMiddleware($middleware, $next), - fn (Request $req): mixed => $handler($req) -); -``` - ---- - -## 5. Database Access - -### Driver - -Raw **PDO** with MySQL (`src/Core/Database/ConnectionFactory.php`). **No ORM**, no query builder (Medoo is referenced in docs but not in actual code). PDO is configured with: -```php -PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, -PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, -PDO::ATTR_EMULATE_PREPARES => false, -``` - -### Prepared Statements — Mandatory - -**All** user-supplied data goes through prepared statements with named parameters: -```php -$stmt = $this->pdo->prepare( - 'UPDATE orders SET external_status_id = :status, updated_at = NOW() WHERE id = :id' -); -$stmt->execute(['status' => $newStatusCode, 'id' => $orderId]); -``` - -For `IN (...)` clauses with a dynamic list, positional `?` placeholders are used and values are bound with `bindValue`: -```php -$placeholders = implode(',', array_fill(0, count($cleanIds), '?')); -$stmt = $this->pdo->prepare('SELECT ... WHERE oi.order_id IN (' . $placeholders . ')'); -foreach ($cleanIds as $index => $orderId) { - $stmt->bindValue($index + 1, $orderId, PDO::PARAM_INT); -} -``` - -Dynamic `ORDER BY` columns are resolved with `match` expressions against a hard-coded allowlist — never interpolated from user input: -```php -$sortColumn = match ($sort) { - 'source_order_id' => 'o.source_order_id', - 'total_with_tax' => 'o.total_with_tax', - default => $effectiveOrderedAtSql, -}; -``` - -### SQL Style - -Multi-line SQL strings use string concatenation only for safe, code-generated SQL fragments (not user data). Named SQL helper methods extract reusable SQL snippets: -```php -private function effectiveStatusSql(string $orderAlias, string $mappingAlias): string -{ - return 'CASE WHEN ' . $orderAlias . '.source = "allegro" AND ...'; -} - -private function effectiveOrderedAtSql(string $orderAlias): string -{ - return 'COALESCE(' . $orderAlias . '.ordered_at, ' . $orderAlias . '.source_created_at, ...)'; -} -``` - -### Error Handling in DB Layer - -Repository methods catch `Throwable` and return safe defaults (empty array, `null`, or `false`) rather than letting exceptions propagate to the controller: -```php -try { - $rows = $this->pdo->query('SELECT ...')->fetchAll(PDO::FETCH_COLUMN); -} catch (Throwable) { - return []; -} -``` - ---- - -## 6. Frontend Patterns - -### View System - -Plain PHP views rendered via `src/Core/View/Template.php`. The `render()` method: -1. Renders the view file with `extract($data)` — data keys become local variables -2. Wraps it in a layout via a second `renderFile()` call, injecting `$content` +**PDO prepared statements only — no ORM, no string concatenation.** ```php -$html = $this->template->render('orders/list', $data, 'layouts/app'); +// Correct +$stmt = $pdo->prepare('SELECT * FROM orders WHERE id = :id'); +$stmt->bindValue(':id', $id, PDO::PARAM_INT); +$stmt->execute(); + +// Never +$pdo->query("SELECT * FROM orders WHERE id = $id"); // forbidden ``` -Two helpers are injected into every view scope: -```php -$e = static fn (mixed $value): string => htmlspecialchars((string) $value, ENT_QUOTES, 'UTF-8'); -$t = static fn (string $key, array $replace = []): string => $translator->get($key, $replace); -``` +- `ATTR_EMULATE_PREPARES = false` (real server-side preparation) +- `ATTR_ERRMODE = ERRMODE_EXCEPTION` +- Parameter type hints: `PDO::PARAM_INT` for integers -**Always use `$e()` for output.** Use `` (raw echo) only when the value is pre-escaped HTML built in the controller. - -### View Structure - -``` -resources/views/ -├── layouts/ -│ ├── app.php # Main authenticated layout (sidebar + topbar) -│ └── auth.php # Login layout -├── components/ -│ ├── table-list.php # Generic paginated table with filters, sorting, column toggle -│ └── order-status-panel.php -├── orders/ -│ ├── list.php -│ └── show.php -├── settings/ -│ └── *.php # One view per settings section -└── shipments/ - └── prepare.php -``` - -Views are **data receivers**, not logic writers. All HTML fragment generation for complex cells (status badges, product previews) happens in controller methods and is passed as pre-built HTML strings with `raw: true` in the table config. - -### Reusable Components - -The `resources/views/components/table-list.php` component accepts a `$tableList` config array and renders a full paginated table with filters, column toggles, and action forms. Include it with: -```php -resolve('components/table-list') ?> -// or via the view data key 'tableList' -``` - -The component reads its own config defensively: -```php -$config = is_array($tableList ?? null) ? $tableList : []; -$rows = is_array($config['rows'] ?? null) ? $config['rows'] : []; -``` - -### SCSS / CSS - -Source: `resources/scss/` -Compiled output: `public/assets/css/` -Build command: `npm run build:assets` (uses Dart Sass) - -``` -resources/scss/ -├── app.scss # Main panel stylesheet -├── login.scss # Auth page stylesheet -└── shared/ - └── _ui-components.scss # Shared UI primitives (imported with @use) -``` - -**Never put styles in view files.** All styles go in SCSS. Cache-busting on the compiled CSS is done via `filemtime()` in the layout: -```php - -``` - -### JavaScript - -No bundler, no TypeScript. Vanilla JavaScript only (ES5 style with `var` in components, ES6 `const/let` allowed in module files). - -**Module pattern:** All JS in views is wrapped in an IIFE to avoid global scope pollution: -```js -(function() { - // component code -})(); -``` - -**UI Alerts and Confirmations:** The custom `window.OrderProAlerts` module (built from `resources/modules/jquery-alerts/`) is used for all confirmations. Never use `window.confirm()` directly. The pattern in table-list: -```js -if (window.OrderProAlerts && typeof window.OrderProAlerts.confirm === 'function') { - window.OrderProAlerts.confirm({ title, message, ... }).then(function(accepted) { - if (!accepted) return; - form.setAttribute('data-confirmed', '1'); - form.submit(); - }); -} -``` - -**DOM targeting:** Use `data-` attributes and `js-` prefixed class names for JS hooks. Never use presentation class names (`.btn--primary`) as JS selectors. - -**localStorage:** Filter state, column visibility, and sidebar collapsed state are persisted in `localStorage` with namespaced keys (e.g. `tableList_/orders/list_orders_filters_open`). - ---- - -## 7. Security +## Security Patterns ### CSRF -All POST forms include a hidden token field: -```html - -``` - -Controllers validate it before processing: ```php -$csrfToken = (string) $request->input('_csrf_token', ''); -if (!Csrf::validate($csrfToken)) { - $_SESSION['order_flash_error'] = $this->translator->get('auth.errors.csrf_expired'); - return Response::redirect('/orders/' . $orderId); -} +// Generate (in controller) +'csrfToken' => Csrf::token() // stores in $_SESSION['_csrf_token'] + +// In view + + +// Validate (in controller) +if (!Csrf::validate((string) $request->input('_token', ''))) { ... } ``` -`Csrf::validate()` uses `hash_equals()` to prevent timing attacks (`src/Core/Security/Csrf.php`). +Field name is always `_token`. Uses `hash_equals()` for timing-safe comparison. -### XSS +### XSS Escaping -Every dynamic value echoed in a view must go through `$e()`: -```php - - -``` - -Pre-built HTML fragments (e.g. status badge HTML assembled in controller) are output raw and must already be escaped internally using `htmlspecialchars(..., ENT_QUOTES, 'UTF-8')`. - -### Secrets Encryption - -Integration credentials (API keys, tokens) are encrypted at rest using AES-256-CBC with HMAC-SHA256 authentication (`src/Modules/Settings/IntegrationSecretCipher.php`). The cipher uses a key derived from `INTEGRATIONS_SECRET` env var. Encrypted values are prefixed with `v1:`. - ---- - -## 8. Error Handling - -### Global Handler - -Unhandled exceptions are caught in `Application::registerErrorHandlers()`: -```php -set_exception_handler(function (Throwable $exception) use ($debug): void { - $this->logger->error('Unhandled exception', [ - 'message' => $exception->getMessage(), - 'file' => $exception->getFile(), - 'line' => $exception->getLine(), - ]); - - $message = $debug ? $exception->getMessage() : 'Internal server error'; - Response::html($message, 500)->send(); -}); -``` - -### Repository Layer - -Repositories return safe fallback values on error and never expose raw exception messages to callers: -```php -try { - // ... -} catch (Throwable $exception) { - return [ - 'items' => [], - 'total' => 0, - 'error' => $exception->getMessage(), // returned in the data, not thrown - ]; -} -``` - -For methods returning `bool`, failure is signaled by returning `false`: -```php -public function updateOrderStatus(...): bool -{ - try { /* ... */ return true; } - catch (Throwable) { return false; } -} -``` - -For methods returning `?array`, failure returns `null`. - -### Cron Layer - -The `CronRunner` catches per-job exceptions, logs them, marks the job as failed, and continues processing the next job: -```php -} catch (Throwable $exception) { - $this->repository->markJobFailed($jobId, $exception->getMessage(), ...); - $this->logger->error('Cron job failed', [ - 'job_id' => $jobId, - 'job_type' => $jobType, - 'error' => $exception->getMessage(), - ]); - $failed++; -} -``` - -### Logger - -`src/Core/Support/Logger.php` — custom file logger writing to `storage/logs/`. Format: -``` -[2026-03-12 14:22:01] ERROR Cron job failed {"job_id":42,"job_type":"allegro_orders_import","error":"..."} -``` - -Methods: `$logger->error(string $message, array $context = [])` and `$logger->info(...)`. Context is encoded as JSON on the same line. Log path is configured via `app.log_path` in `config/app.php`. - ---- - -## 9. Testing - -### Framework - -PHPUnit 11.x. Config: `phpunit.xml` at project root. - -```bash -composer test # vendor/bin/phpunit -c phpunit.xml --testdox -``` - -Settings: -- `failOnWarning="true"` — warnings are test failures -- `failOnRisky="true"` — risky tests fail -- `executionOrder="depends,defects"` — dependency-aware ordering - -### Test File Location - -Tests live in `tests/` (PSR-4 namespace `Tests\`), mirroring the `src/` structure: -``` -tests/ -├── bootstrap.php -└── Unit/ - ├── Cron/ - │ └── CronJobTypeTest.php - └── Settings/ - └── OrderStatusMappingRepositoryTest.php -``` - -**Note:** The active `tests/` directory currently contains only `bootstrap.php`. The working test files exist in `archive/2026-03-02_users-only-reset/tests/` — these are the reference patterns to follow when adding new tests. - -### Test Class Pattern +All user-controlled output escaped with `$e()` helper (available in all views): ```php - htmlspecialchars((string)$v, ENT_QUOTES, 'UTF-8'); -namespace Tests\Unit\Settings; +// Usage + + +``` -use App\Modules\Settings\OrderStatusMappingRepository; -use PDO; -use PHPUnit\Framework\Attributes\CoversClass; -use PHPUnit\Framework\TestCase; +**Never** output raw variables without `$e()`. -#[CoversClass(OrderStatusMappingRepository::class)] -final class OrderStatusMappingRepositoryTest extends TestCase -{ - private PDO $pdo; - private OrderStatusMappingRepository $repository; +### Session - protected function setUp(): void - { - $this->pdo = new PDO('sqlite::memory:'); - $this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); - $this->pdo->exec('CREATE TABLE ...'); // schema DDL - $this->repository = new OrderStatusMappingRepository($this->pdo); - } +Configured with: `cookie_httponly=true`, `cookie_secure=true`, `cookie_samesite=Lax`, `use_strict_mode=true`. +Access via `Session::get()` / `Session::set()` helpers — not raw `$_SESSION` in business logic. - public function testReplaceAndReadMappingsForIntegration(): void - { - $this->repository->replaceForIntegration(10, [...]); - $rows = $this->repository->listByIntegration(10); - self::assertArrayHasKey('new', $rows); - self::assertSame('completed', $rows['paid']['orderpro_status_code']); +## Controller Pattern + +```php +final class OrdersController { + public function __construct( + private readonly Template $template, + private readonly Translator $translator, + private readonly OrdersRepository $orders, + // ... + ) {} + + public function index(Request $request): Response { + // 1. Parse & validate input + $filters = ['search' => trim((string) $request->input('search', ''))]; + + // 2. Call repository + $result = $this->orders->paginate($filters); + + // 3. Prepare view data + $rows = array_map(fn($row) => $this->toTableRow($row), $result['items']); + + // 4. Render + return Response::html( + $this->template->render('orders/index', ['rows' => $rows], 'layouts/app') + ); } } ``` -Key patterns: -- Use `#[CoversClass(ClassName::class)]` PHP 8 attribute on every test class -- SQLite in-memory database for repository tests (no real DB connection needed) -- Test method names start with `test` and describe behavior: `testReplaceAndReadMappingsForIntegration` -- Use `self::assertSame()` (strict) not `assertEquals()` -- Classes and methods are `final` -- No mocking framework observed — tests use real classes with SQLite in-memory +## View Pattern -### What to Test +Views use two magic helpers injected by `Template::renderFile()`: +- `$e($value)` — HTML-escape +- `$t($key, $replace)` — translate -Repository tests test read/write behavior against an in-memory SQLite DB. Value object / enum tests verify constants and helper methods. No integration tests against the real MySQL database. - ---- - -## 10. Code Quality Tools - -### SonarQube - -Configured via `sonar-project.properties`: -``` -sonar.sources=src,resources/views,routes -sonar.tests=tests -sonar.exclusions=archive/**,node_modules/**,vendor/**,public/assets/**,storage/** -sonar.php.version=8.1 -``` -Runs against an external SonarQube server at `https://sonar.project-pro.pl`. - -### PHPUnit (Coverage Source) - -`phpunit.xml` configures `src/` as the source for coverage analysis. - -### No Local Static Analysis Config - -No `phpstan.neon`, `.phpcs.xml`, or `psalm.xml` are present. SonarQube is the sole static analysis tool. PHP_CS_Fixer and PHPStan are not in `composer.json`. - ---- - -## 11. Import / Use Statement Organization - -Use statements are grouped and ordered: -1. Internal `App\Core\*` namespaces -2. Internal `App\Modules\*` namespaces -3. PHP built-in classes (`PDO`, `DateTimeImmutable`, `RuntimeException`, `Throwable`) - -Each `use` on its own line. No `use` grouping with `{}`. - ---- - -## 12. Configuration - -App config is split by concern in `config/`: -- `config/app.php` — application settings (paths, locale, session, logging, cron) -- `config/database.php` — DB connection parameters - -The `Application::config(string $key, mixed $default)` method resolves dot-notation keys: +Layout composition: ```php -$app->config('app.integrations.secret', '') -$app->config('database.host') +$this->template->render('orders/index', $data, 'layouts/app') +// renders views/orders/index.php, wraps in views/layouts/app.php via $content ``` -Environment variables are loaded via `src/Core/Support/Env.php` and read in config files. +## UI Rules ---- +### Alerts & Confirmations +- **Always** use `window.OrderProAlerts.confirm({message, onConfirm})` from `jquery-alerts.js` +- **Never** use native `alert()` or `confirm()` -*Convention analysis: 2026-03-12* +### CSS / SCSS +- All styles go in `resources/scss/` — never inline `