174 lines
6.1 KiB
Markdown
174 lines
6.1 KiB
Markdown
|
|
# Code Quality Improvement Checklist
|
||
|
|
|
||
|
|
Use this checklist to track progress on the refactoring plan.
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Phase 1: Quick Wins ✅ COMPLETE
|
||
|
|
|
||
|
|
### 1.1 Add Mapping Helper Methods ✅
|
||
|
|
|
||
|
|
- [x] Add `getWhmcsClientIdOrThrow()` to `MappingsService`
|
||
|
|
- [x] Add `getMappingOrThrow()` to `MappingsService`
|
||
|
|
- [ ] Add unit tests for new methods
|
||
|
|
- [x] Update `index.ts` exports if needed (N/A - no index.ts)
|
||
|
|
|
||
|
|
### 1.2 Refactor WHMCS Mapping Lookups ✅
|
||
|
|
|
||
|
|
- [x] `apps/bff/src/modules/billing/billing.controller.ts` (4 occurrences)
|
||
|
|
- [x] `apps/bff/src/modules/billing/services/invoice-retrieval.service.ts` (1 occurrence)
|
||
|
|
- [x] Remove private `getUserMapping()` method
|
||
|
|
- [x] `apps/bff/src/modules/users/infra/user-profile.service.ts` (2 occurrences)
|
||
|
|
- [x] `apps/bff/src/modules/subscriptions/subscriptions.service.ts` (3 occurrences)
|
||
|
|
- [x] `apps/bff/src/modules/subscriptions/sim-management/services/esim-management.service.ts` (1 occurrence)
|
||
|
|
- [x] `apps/bff/src/modules/subscriptions/sim-management/services/sim-cancellation.service.ts` (2 occurrences)
|
||
|
|
- [x] `apps/bff/src/modules/subscriptions/sim-management/services/sim-topup.service.ts` (1 occurrence)
|
||
|
|
- [x] `apps/bff/src/modules/subscriptions/sim-order-activation.service.ts` (1 occurrence)
|
||
|
|
- [x] `apps/bff/src/modules/auth/application/auth.facade.ts` (1 occurrence)
|
||
|
|
- [ ] Run tests after each file change
|
||
|
|
|
||
|
|
### 1.3 Rename `getErrorMessage` Functions ✅
|
||
|
|
|
||
|
|
**Step 1: Add new function names as aliases**
|
||
|
|
|
||
|
|
- [x] In `apps/bff/src/core/utils/error.util.ts`: Add `extractErrorMessage` as alias
|
||
|
|
- [x] In `packages/domain/common/errors.ts`: Add `getMessageForErrorCode` as alias
|
||
|
|
- [x] Update exports in both locations
|
||
|
|
|
||
|
|
**Step 2: Update all imports (gradual migration)**
|
||
|
|
|
||
|
|
- [x] Old names kept as deprecated aliases for backwards compatibility
|
||
|
|
- [ ] Update imports incrementally (can be done over time)
|
||
|
|
|
||
|
|
**Step 3: Remove old function names**
|
||
|
|
|
||
|
|
- [ ] Remove `getErrorMessage` from `error.util.ts` (after migration complete)
|
||
|
|
- [ ] Remove `getErrorMessage` from `errors.ts` (after migration complete)
|
||
|
|
- [ ] Run full test suite
|
||
|
|
|
||
|
|
### 1.4 Remove Unused Status Wrapper Methods ✅
|
||
|
|
|
||
|
|
**Audit usage:**
|
||
|
|
|
||
|
|
- [x] `invoice-retrieval.service.ts` → `getUnpaidInvoices()` - UNUSED, REMOVED
|
||
|
|
- [x] `invoice-retrieval.service.ts` → `getOverdueInvoices()` - UNUSED, REMOVED
|
||
|
|
- [x] `invoice-retrieval.service.ts` → `getPaidInvoices()` - UNUSED, REMOVED
|
||
|
|
- [x] `invoice-retrieval.service.ts` → `getCancelledInvoices()` - UNUSED, REMOVED
|
||
|
|
- [x] `invoice-retrieval.service.ts` → `getCollectionsInvoices()` - UNUSED, REMOVED
|
||
|
|
- [x] `invoice-retrieval.service.ts` → `hasInvoices()` - UNUSED, REMOVED
|
||
|
|
- [x] `invoice-retrieval.service.ts` → `getInvoiceCountByStatus()` - UNUSED, REMOVED
|
||
|
|
- [x] `subscriptions.service.ts` → `getSuspendedSubscriptions()` - UNUSED, REMOVED
|
||
|
|
- [x] `subscriptions.service.ts` → `getCancelledSubscriptions()` - UNUSED, REMOVED
|
||
|
|
- [x] `subscriptions.service.ts` → `getPendingSubscriptions()` - UNUSED, REMOVED
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Phase 2: Medium-Term Improvements ✅ COMPLETE
|
||
|
|
|
||
|
|
### 2.1 Create Error Handling Utility ✅
|
||
|
|
|
||
|
|
- [x] Create `apps/bff/src/core/utils/error-handler.util.ts`
|
||
|
|
- [x] Add `withErrorHandling()` function
|
||
|
|
- [x] Add `withErrorSuppression()` function
|
||
|
|
- [x] Add `withErrorLogging()` function
|
||
|
|
- [ ] Add unit tests
|
||
|
|
- [ ] Export from `apps/bff/src/core/utils/index.ts` (optional)
|
||
|
|
|
||
|
|
### 2.2 Refactor Services to Use Error Handling Utility (Partial) ✅
|
||
|
|
|
||
|
|
**High Priority:**
|
||
|
|
|
||
|
|
- [x] `invoice-retrieval.service.ts` - Refactored to use `withErrorHandling`
|
||
|
|
|
||
|
|
**Medium Priority (not yet done):**
|
||
|
|
|
||
|
|
- [ ] `subscriptions.service.ts` (12 blocks)
|
||
|
|
- [ ] `user-profile.service.ts` (6 blocks)
|
||
|
|
- [ ] `signup-workflow.service.ts` (8 blocks)
|
||
|
|
- [ ] `password-workflow.service.ts` (4 blocks)
|
||
|
|
- [ ] `whmcs-link-workflow.service.ts` (5 blocks)
|
||
|
|
|
||
|
|
### 2.3 Consolidate WHMCS Connection Methods ✅
|
||
|
|
|
||
|
|
- [x] Update `WhmcsRequestOptions` interface with `highPriority` option
|
||
|
|
- [x] Refactor `makeRequest()` to handle both cases
|
||
|
|
- [x] Deprecate `makeHighPriorityRequest()` (don't remove yet)
|
||
|
|
- [ ] Update callers of `makeHighPriorityRequest()` (not required, deprecated method delegates)
|
||
|
|
- [ ] Run integration tests
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Phase 3: Long-Term Improvements (Partial)
|
||
|
|
|
||
|
|
### 3.1 Split SignupWorkflowService ✅
|
||
|
|
|
||
|
|
**Create new files:**
|
||
|
|
|
||
|
|
- [x] `signup.types.ts`
|
||
|
|
- [x] `signup-validation.service.ts`
|
||
|
|
- [x] `signup-account-resolver.service.ts`
|
||
|
|
- [x] `index.ts` (exports)
|
||
|
|
|
||
|
|
**Migration:**
|
||
|
|
|
||
|
|
- [x] Move validation logic to `signup-validation.service.ts`
|
||
|
|
- [x] Move account resolution to `signup-account-resolver.service.ts`
|
||
|
|
- [x] Refactor `signup-workflow.service.ts` to use new services
|
||
|
|
- [x] Update `auth.module.ts` with new providers
|
||
|
|
- [ ] Update tests
|
||
|
|
- [x] Keep `signup-workflow.service.ts` as orchestrator (delegating to new services)
|
||
|
|
|
||
|
|
### 3.2 Split UserProfileService (DEFERRED)
|
||
|
|
|
||
|
|
**Create new files:**
|
||
|
|
|
||
|
|
- [ ] `dashboard.service.ts`
|
||
|
|
- [ ] `whmcs-custom-fields.util.ts`
|
||
|
|
|
||
|
|
**Migration:**
|
||
|
|
|
||
|
|
- [ ] Extract `getUserSummary()` to `DashboardService`
|
||
|
|
- [ ] Extract custom field logic to utility
|
||
|
|
- [ ] Update module providers
|
||
|
|
- [ ] Update tests
|
||
|
|
|
||
|
|
### 3.3 Evaluate WhmcsService Facade (DEFERRED)
|
||
|
|
|
||
|
|
**Analysis:**
|
||
|
|
|
||
|
|
- [ ] List all direct usages of `WhmcsService`
|
||
|
|
- [ ] Determine if cross-cutting concerns are needed
|
||
|
|
- [ ] Make go/no-go decision
|
||
|
|
|
||
|
|
### 3.4 Dead Code Audit (DEFERRED)
|
||
|
|
|
||
|
|
**Tools:**
|
||
|
|
|
||
|
|
- [ ] Run `ts-prune` to find unused exports
|
||
|
|
- [ ] Review test coverage reports
|
||
|
|
|
||
|
|
### 3.5 Create LoggableService Base Class (DEFERRED - Optional)
|
||
|
|
|
||
|
|
- [ ] Team decision on using inheritance
|
||
|
|
- [ ] If yes: Create base class
|
||
|
|
- [ ] If yes: Migrate services incrementally
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Final Verification
|
||
|
|
|
||
|
|
- [ ] All unit tests passing
|
||
|
|
- [ ] All integration tests passing
|
||
|
|
- [ ] Manual smoke testing complete
|
||
|
|
- [x] No new linter errors (verified with TypeScript compiler)
|
||
|
|
- [x] Documentation updated
|
||
|
|
- [ ] PR reviewed and approved
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Notes
|
||
|
|
|
||
|
|
| Date | Note |
|
||
|
|
| -------- | -------------------------------------------------------------------------------------------------------------------------------------- |
|
||
|
|
| Dec 2025 | Phase 1 and Phase 2 completed. Phase 3.1 (SignupWorkflowService split) completed. Remaining Phase 3 items deferred for future sprints. |
|