diff --git a/IMPROVEMENTS.md b/IMPROVEMENTS.md index 1fbc019b..2ca95fbe 100644 --- a/IMPROVEMENTS.md +++ b/IMPROVEMENTS.md @@ -46,52 +46,11 @@ ### High Priority -- **Fix excessive `as any` type casts bypassing type safety** → [Issue #29](https://github.com/mchestr/plex-manager/issues/29) - - **Files affected**: - - `actions/maintenance.ts:75, 108` - `criteria: validated.criteria as any` - - `lib/maintenance/scanner.ts:230` - `const criteria = rule.criteria as any` - - `lib/maintenance/rule-evaluator.ts:219` - `return (item as any)[fieldKey]` - - **Problem**: Prisma's JSON type doesn't recognize Zod's validated types, using `as any` defeats TypeScript's purpose - - **Fix**: Use proper Prisma.JsonValue casting or Record for dynamic access - - **Impact**: Type safety violations could cause runtime errors - - **Reference**: TypeScript strict mode enabled in project (CLAUDE.md) - -- **Fix dynamic require() in production code** → [Issue #30](https://github.com/mchestr/plex-manager/issues/30) - - **File**: `lib/maintenance/rule-evaluator.ts:76` - - **Code**: `const { migrateLegacyRuleCriteria } = require('@/lib/validations/maintenance')` - - **Problem**: Using `require()` in ESM project causes potential tree-shaking issues and runtime errors - - **Fix**: Move to top-level import or make migration explicit - - **Impact**: Could break in production builds, prevents proper code optimization - -- **Fix unguarded dynamic object property access** → [Issue #31](https://github.com/mchestr/plex-manager/issues/31) - - **File**: `lib/maintenance/rule-evaluator.ts:219` - - **Code**: `return (item as any)[fieldKey]` - - **Problem**: Dynamic property access with `as any` bypasses type checking - - **Fix**: Use `Record` type instead of `any` - - **Impact**: No type safety for field access, could access undefined properties +- ### Medium Priority -- **Complete or remove incomplete worker implementation** → [Issue #32](https://github.com/mchestr/plex-manager/issues/32) - - **File**: `lib/maintenance/worker.ts` - - **Lines**: 32-46 (scanner returns `{ candidatesFound: 0 }`), 82-96 (deleter returns `{ deletedCount: 0 }`) - - **Problem**: Worker file exists but functions are not implemented, just return empty results - - **Fix**: Either complete implementation or remove file if not needed yet - - **Impact**: Dead code in production, could confuse developers - -- **Add error context to scanner error messages** → [Issue #33](https://github.com/mchestr/plex-manager/issues/33) - - **File**: `lib/maintenance/scanner.ts:246-249` - - **Problem**: Throws generic errors without rule/library context for debugging - - **Fix**: Include rule name, ID, and media type in error messages - - **Example**: `throw new Error(\`${mediaType} rules must specify libraryIds. Rule: ${rule.name} (${ruleId})\`)` - - **Impact**: Harder to debug issues in production - -- **Replace magic numbers with named constants** → [Issue #34](https://github.com/mchestr/plex-manager/issues/34) - - **File**: `lib/maintenance/scanner.ts` - - **Lines**: 72 (`length: 10000`), 284 (`itemsScanned % 10 === 0`) - - **Problem**: Hardcoded pagination limits and progress intervals reduce maintainability - - **Fix**: Extract to named constants (e.g., `MAX_LIBRARY_ITEMS`, `PROGRESS_REPORT_INTERVAL`) - - **Impact**: Harder to adjust thresholds, unclear intent +- ### Low Priority @@ -125,57 +84,9 @@ - **Why**: Ensures consistent styling, behavior, and accessibility across the app - **When adding new UI**: First check if a component exists in `components/ui/` before creating anything new -- **Split large maintenance components exceeding file size guidelines** → [Issue #35](https://github.com/mchestr/plex-manager/issues/35) - - **Files affected**: - - `components/maintenance/condition-value-input.tsx` - 486 lines (guideline: <200-300) - - `app/admin/maintenance/candidates/page.tsx` - 381 lines - - `app/admin/maintenance/rules/page.tsx` - 318 lines - - **Problem**: Large components violate Single Responsibility Principle and file size guidelines - - **Fix for condition-value-input.tsx**: Extract individual input types (StringInput, NumberInput, DateInput, etc.) into separate files - - **Fix for candidates page**: Split into `CandidateList`, `CandidateFilters`, `CandidateActions` components - - **Fix for rules page**: Extract `RuleList`, `RuleFilters`, `RuleActions` components - - **Impact**: Harder to test, maintain, and understand at a glance - - **Reference**: CLAUDE.md Component Design Philosophy (200-300 line limit) - -- **Use styled UI component instead of raw textarea** → [Issue #36](https://github.com/mchestr/plex-manager/issues/36) - - **File**: `app/admin/maintenance/rules/new/page.tsx:142` - - **Problem**: Using raw `