Appearance
Backend Code Review Report
Date: 2026-03-02 Scope: Full backend code review of the Partizap PHP API Branch: develop (commit 564eb3f)
Summary
A comprehensive code review identified 14 issues across the backend codebase. After investigation, 3 required no changes (already correct). The remaining 11 issues were resolved in a single commit covering 7 logical groups. All fixes pass PHPStan level 8, all 48 tests, and PHP-CS-Fixer with zero violations.
| Category | Found | Fixed | No change needed |
|---|---|---|---|
| Performance | 1 | 1 | 0 |
| Consistency | 2 | 2 | 0 |
| Type safety | 4 | 4 | 0 |
| Security | 1 | 1 | 0 |
| Data integrity | 1 | 1 | 0 |
| Convention | 1 | 1 | 0 |
| Configuration | 1 | 1 | 0 |
| Already correct | 3 | 0 | 3 |
| Total | 14 | 11 | 3 |
Issues Fixed
1. DI container cannot compile in production
Severity: Performance Files: config/container.php
Problem: Every request rebuilt the PHP-DI container because 21 closures captured $settings via use($settings), preventing PHP-DI compilation. The settings key was already registered in the container at line 112.
Fix: Replaced all use ($settings) closures with $c->get('settings') (adding ContainerInterface $c parameter where missing). Enabled container compilation when APP_DEBUG=false. The var/cache/ directory was already in .gitignore.
Impact: Eliminates container rebuild overhead on every request in production.
2. Inconsistent User response shapes
Severity: Consistency Files: app/Actions/Auth/MeAction.php, app/Actions/Admin/UpdateUserAction.php
Problem: GET /auth/me and PUT /admin/users/{id} returned user data without business_profile, while GET /vendor/me and GET /store/sellers/{id} included it. Frontend received inconsistent response shapes for the same entity.
Fix: Added BusinessProfileRepositoryInterface to both actions and conditionally included business_profile when present (matching the pattern in GetMyProfileAction).
Impact: All user-returning endpoints now include business_profile when the user has one.
3. Raw strings for Message.type
Severity: Type safety Files: app/Domain/Enum/MessageType.php (new), app/Domain/Entity/Message.php, app/Actions/Vendor/SendMessageAction.php, app/Actions/Vendor/SendImageMessageAction.php, app/Application/Service/SystemMessageService.php
Problem: Message.type used raw strings ('text', 'image', 'system'), allowing invalid values.
Fix: Created MessageType backed enum with cases Text, Image, System. Updated the entity column, property, constructor, getter, and toArray(). Updated all 3 callers to use enum cases.
4. Raw strings for Report.status
Severity: Type safety Files: app/Domain/Enum/ReportStatus.php (new), app/Domain/Entity/Report.php
Problem: Report.status used raw strings ('pending', 'resolved', 'rejected').
Fix: Created ReportStatus backed enum. Updated entity column, property, default value, setter, getter, and toArray().
5. Raw strings for Payment.status and Payment.paymentType
Severity: Type safety Files: app/Domain/Enum/PaymentStatus.php (new), app/Domain/Enum/PaymentType.php (new), app/Domain/Entity/Payment.php
Problem: Payment.status and Payment.paymentType used raw strings.
Fix: Created PaymentStatus (Pending, Completed, Failed, Cancelled) and PaymentType (Promotion, Featured, Subscription) backed enums. Updated entity columns, properties, constructor, setters, getters, and toArray().
6. Email-based rate limiting bypassable
Severity: Security Files: config/container.php
Problem: Login and forgot-password rate limits used email-based key extraction. An attacker could bypass per-email buckets by cycling through different emails, getting 5 fresh attempts per email.
Fix: Removed the custom keyExtractor closures from rate_limit.login and rate_limit.forgot_password. Both now use the default IP-based key from RateLimitMiddleware. An IP gets N total attempts regardless of which emails it tries.
Impact: Rate limits are now purely IP-based. Removed unused ServerRequestInterface import.
7. Admin product deletion not transactional
Severity: Data integrity Files: app/Actions/Admin/DeleteProductAction.php
Problem: DeleteProductAction ran broadcast, delete, and audit log without a transaction. If delete failed after broadcast, users would see a "deleted" system message for a still-existing product.
Fix: Added EntityManagerInterface dependency. Wrapped delete + audit log in $this->em->wrapInTransaction(). Moved the Centrifugo broadcast after the transaction (best-effort external call).
8. RemoveFavoriteAction bypasses JsonResponder
Severity: Convention Files: app/Actions/Vendor/RemoveFavoriteAction.php
Problem: The only action that returned a raw PSR-7 response ($response->withStatus(204)->withHeader(...)) instead of using JsonResponder.
Fix: Added JsonResponder dependency. Replaced raw response with $this->responder->respond(null, 204).
9. S3 bucket fallback undocumented
Severity: Configuration Files: config/app.php, .env.example
Problem: S3_PUBLIC_BUCKET and S3_PRIVATE_BUCKET silently fell back to S3_BUCKET, masking potential misconfiguration.
Fix: Added a comment in config/app.php documenting the fallback behavior. Added S3_PRIVATE_BUCKET, S3_PUBLIC_BUCKET, and S3_CDN_BASE_URL entries to .env.example with explanatory comments. No breaking change -- current environments only set S3_BUCKET.
Issues Confirmed OK (No Changes)
10. Missing index on products.seller_id
Investigation: Index IDX_PRODUCTS_SELLER already exists, created in migration Version20260206120000.
11. SearchLog silently swallows exceptions
Investigation: The catch block already calls $this->logger->warning() -- correct pattern for a non-critical logging feature.
12. WatermarkService font path is relative
Investigation: Path is already absolute via dirname(__DIR__) . '/resources/fonts/Roboto-Regular.ttf' in config/container.php.
Verification
| Check | Result |
|---|---|
composer analyse (PHPStan level 8) | No errors (260 files) |
composer test (PHPUnit) | 48 tests, 122 assertions, all passing |
composer cs-fix (PHP-CS-Fixer) | 0 files fixed (280 files scanned) |
Documentation Updates
The following documentation was updated in the same session:
docs/frontend-api-reference.md:- Added full Admin Endpoints section: dashboard stats, user management (list/get/update/delete), product moderation (list/pending/get/approve/reject/delete), reference data CRUD (car makes/models/generations/modifications, categories, geography), with request/response shapes and error codes
- Added
business_profileobject to the User Object section - Updated
GET /auth/meresponse example to showbusiness_profile - Added new enums to the Enums table: MessageType, ReportStatus, PaymentStatus, PaymentType, FuelType, Transmission, Drivetrain
- Updated rate limiting table: login and forgot-password are now per-IP (not per-email)