Assist_Design/docs/development/refactoring/code-quality-checklist.md

174 lines
6.1 KiB
Markdown
Raw Normal View History

# 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. |