150 lines
6.2 KiB
Markdown
150 lines
6.2 KiB
Markdown
---
|
|
phase: 68-code-deduplication-refactor
|
|
plan: 01
|
|
subsystem: infra
|
|
tags: [refactor, deduplication, trait, static-helper]
|
|
|
|
requires: []
|
|
provides:
|
|
- SslCertificateResolver — single source for CA bundle path
|
|
- ToggleableRepositoryTrait — reusable toggle pattern for repositories
|
|
- RedirectPathResolver — single source for redirect path validation
|
|
- ReceiptService — single source for receipt issuance logic
|
|
affects: [future integrations, new repositories with toggle, new controllers with redirects]
|
|
|
|
tech-stack:
|
|
added: []
|
|
patterns: [static resolver classes, repository traits]
|
|
|
|
key-files:
|
|
created:
|
|
- src/Core/Http/SslCertificateResolver.php
|
|
- src/Core/Http/ToggleableRepositoryTrait.php
|
|
- src/Core/Http/RedirectPathResolver.php
|
|
- src/Modules/Accounting/ReceiptService.php
|
|
- src/Modules/Accounting/ReceiptIssueException.php
|
|
modified:
|
|
- src/Modules/Settings/AllegroApiClient.php
|
|
- src/Modules/Settings/ApaczkaApiClient.php
|
|
- src/Modules/Settings/ShopproApiClient.php
|
|
- src/Modules/Settings/AllegroOAuthClient.php
|
|
- src/Modules/Shipments/AllegroTrackingService.php
|
|
- src/Modules/Shipments/InpostShipmentService.php
|
|
- src/Modules/Shipments/InpostTrackingService.php
|
|
- src/Modules/Settings/EmailMailboxRepository.php
|
|
- src/Modules/Settings/EmailTemplateRepository.php
|
|
- src/Modules/Settings/ReceiptConfigRepository.php
|
|
- src/Modules/Automation/AutomationRepository.php
|
|
- src/Modules/Settings/AllegroIntegrationController.php
|
|
- src/Modules/Settings/ApaczkaIntegrationController.php
|
|
- src/Modules/Settings/InpostIntegrationController.php
|
|
- src/Modules/Accounting/ReceiptController.php
|
|
- src/Modules/Automation/AutomationService.php
|
|
- src/Modules/Cron/CronHandlerFactory.php
|
|
- routes/web.php
|
|
|
|
key-decisions:
|
|
- "Static resolver classes for stateless utilities (SSL, redirect)"
|
|
- "Trait for toggle pattern — repos keep thin public wrapper for backward compat"
|
|
- "ReceiptService as full service class (not static) — needs DI dependencies"
|
|
|
|
patterns-established:
|
|
- "Stateless utility → final class with static methods in Core/Http/"
|
|
- "Shared repo behavior → trait in Core/Http/ with $this->db/$this->pdo access"
|
|
|
|
duration: ~25min
|
|
started: 2026-04-03T19:10:00Z
|
|
completed: 2026-04-03T19:35:00Z
|
|
---
|
|
|
|
# Phase 68 Plan 01: Code Deduplication Refactor Summary
|
|
|
|
**Extracted 3 shared utilities + 1 service from 17+ duplicated methods across codebase, eliminating ~250 lines of copy-pasted code**
|
|
|
|
## Performance
|
|
|
|
| Metric | Value |
|
|
|--------|-------|
|
|
| Duration | ~25min |
|
|
| Started | 2026-04-03T19:10:00Z |
|
|
| Completed | 2026-04-03T19:35:00Z |
|
|
| Tasks | 3 completed (+ 1 pre-plan ReceiptService) |
|
|
| Files modified | 18 |
|
|
| Delegation | 3 tasks via Claude Agent (parallel) |
|
|
|
|
## Acceptance Criteria Results
|
|
|
|
| Criterion | Status | Notes |
|
|
|-----------|--------|-------|
|
|
| AC-1: getCaBundlePath eliminated from 7 classes | Pass | 0 occurrences remain, SslCertificateResolver is sole source |
|
|
| AC-2: toggleStatus consolidated in trait | Pass | 4 repos use ToggleableRepositoryTrait, controllers unchanged |
|
|
| AC-3: resolveRedirectPath consolidated | Pass | 0 occurrences remain, RedirectPathResolver is sole source |
|
|
|
|
## Accomplishments
|
|
|
|
- Extracted `SslCertificateResolver` replacing 7 identical `getCaBundlePath()` copies
|
|
- Extracted `ToggleableRepositoryTrait` used by 4 repositories (Automation, EmailMailbox, EmailTemplate, ReceiptConfig)
|
|
- Extracted `RedirectPathResolver` replacing 3 identical `resolveRedirectPath()` copies
|
|
- Extracted `ReceiptService` consolidating receipt issuance from ReceiptController + AutomationService (pre-plan bugfix that prompted this phase)
|
|
- Fixed receipt datetime bug: `date('Y-m-d')` → `date('Y-m-d H:i:s')` in automation path
|
|
- Migration 000077: `sale_date` DATE→DATETIME + backfill old receipts from `created_at`
|
|
|
|
## Files Created/Modified
|
|
|
|
| File | Change | Purpose |
|
|
|------|--------|---------|
|
|
| `src/Core/Http/SslCertificateResolver.php` | Created | CA bundle path resolution (was in 7 classes) |
|
|
| `src/Core/Http/ToggleableRepositoryTrait.php` | Created | Shared toggle active/inactive pattern |
|
|
| `src/Core/Http/RedirectPathResolver.php` | Created | Redirect path validation (was in 3 controllers) |
|
|
| `src/Modules/Accounting/ReceiptService.php` | Created | Centralized receipt issuance logic |
|
|
| `src/Modules/Accounting/ReceiptIssueException.php` | Created | Dedicated exception for receipt errors |
|
|
| `database/migrations/20260403_000077_*` | Created | sale_date DATETIME + backfill |
|
|
| 7x API/Tracking clients | Modified | getCaBundlePath → SslCertificateResolver |
|
|
| 4x Repositories | Modified | toggleActive → ToggleableRepositoryTrait |
|
|
| 3x Integration controllers | Modified | resolveRedirectPath → RedirectPathResolver |
|
|
| `ReceiptController.php` | Modified | Delegates to ReceiptService |
|
|
| `AutomationService.php` | Modified | Delegates to ReceiptService, removed 7 private methods |
|
|
| `CronHandlerFactory.php` | Modified | Wires ReceiptService |
|
|
| `routes/web.php` | Modified | Wires ReceiptService |
|
|
|
|
## Deviations from Plan
|
|
|
|
### Summary
|
|
|
|
| Type | Count | Impact |
|
|
|------|-------|--------|
|
|
| Scope additions | 1 | ReceiptService extracted pre-plan as bugfix response |
|
|
| Auto-fixed | 0 | None |
|
|
| Deferred | 0 | None |
|
|
|
|
**Total impact:** ReceiptService was essential — it fixed the datetime bug AND eliminated the largest duplication case.
|
|
|
|
## Re-analysis Results (Post-Refactor)
|
|
|
|
Remaining duplications found:
|
|
|
|
| Priority | Issue | Copies | Recommendation |
|
|
|----------|-------|--------|----------------|
|
|
| HIGH | validateCsrf() | 6 | CsrfValidator service |
|
|
| MEDIUM | isActive filter closure | 3 | Utility function |
|
|
| LOW | array_values(array_filter()) | 12+ | ArrayHelper |
|
|
| LOW | date instantiation pattern | 8+ | Monitor only |
|
|
| LOW | array_map('intval') | 4 | Monitor only |
|
|
|
|
## Next Phase Readiness
|
|
|
|
**Ready:**
|
|
- Core/Http/ established as location for shared utilities
|
|
- Pattern proven: static resolvers + traits work cleanly with existing architecture
|
|
- Re-analysis baseline documented for plan 68-02
|
|
|
|
**Concerns:**
|
|
- validateCsrf duplication is security-sensitive — should be next priority
|
|
- ToggleableRepositoryTrait uses `$this->pdo` — verify consistency across repos
|
|
|
|
**Blockers:** None
|
|
|
|
---
|
|
*Phase: 68-code-deduplication-refactor, Plan: 01*
|
|
*Completed: 2026-04-03*
|