6.1 KiB
6.1 KiB
Technical Concerns & Debt
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 |
Planned fix: Phase 68 (Code Deduplication Refactor) — deferred, never started.
SonarQube Issues (new code since 2026-03-28)
Total: 174 issues (BLOCKER=1, CRITICAL=47, MAJOR=110, MINOR=16)
| 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 <output> instead of <span role="status"> |
Note: SonarQube scan not run for phases 105–108 — baseline may be stale.
Breaking Changes
| Change | Phase | Impact | Migration |
|---|---|---|---|
| Delivery status group keys przeniesione do DB | Phase 108 (2026-04-27) | Stare reguły automation z kluczami registered, courier_pickup, dropped_at_point, unclaimed, picked_up przestają działać |
Operatorzy muszą ręcznie odtworzyć reguły w UI |
SHIPMENT_STATUS_OPTION_MAP usunięty |
Phase 108 (2026-04-27) | AutomationService porównuje klucze statusów bezpośrednio z DB |
Brak wpływu po odtworzeniu reguł |
_csrf_token → _token |
Phase 105 (2026-04-19) | Stare nazwy pól formularzy | Sprawdzić czy nie ma starych referencji _csrf_token w widokach |
Known Bugs & Issues
| 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 |
Deferred Indexes (Phase 106)
After Phase 106 (customer return alert), two indexes should be added before dataset exceeds ~50k orders:
-- 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);
These support the correlated subquery in OrdersRepository used for return-risk detection.
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 |
Architecture Concerns
| 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)
SslCertificateResolver— pattern duplicated across multiple API client filesToggleableRepositoryTrait— mixes query building + togglingRedirectPathResolver— similar redirect logic in 4+ controllersReceiptServicevs accounting logic — overlapping responsibilities
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 |
Performance Risks
- Correlated subquery for return-risk (per-row COUNT) — slow at >50k orders without
INDEX-106-01 - N+1 potential in order details —
findDetails()queries orders + addresses + items separately (not verified as actual problem) - Cron queue growth — no exponential backoff if queue grows; may pile up on slow syncs