Skip to content

Commit b8e199b

Browse files
dannysmithdannysmith
committed
Phase 4
1 parent fe890f1 commit b8e199b

File tree

2 files changed

+141
-8
lines changed

2 files changed

+141
-8
lines changed

‎docs/developer/architecture-guide.md‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,65 @@ Extract to `src/hooks/` when:
136136
3. Same logic needed in multiple components
137137
4. Manages side effects (subscriptions, timers, etc.)
138138

139+
#### Shared File Processing Module
140+
141+
The `src/lib/files/` module demonstrates good module organization for shared business logic:
142+
143+
```
144+
files/
145+
├── index.ts # Public API exports
146+
├── types.ts # TypeScript interfaces
147+
├── constants.ts # IMAGE_EXTENSIONS
148+
├── fileProcessing.ts # Core business logic
149+
└── fileProcessing.test.ts # Comprehensive tests
150+
```
151+
152+
**Core Function: `processFileToAssets()`**
153+
154+
Centralizes file copying logic with configurable behavior via strategy pattern:
155+
156+
```typescript
157+
import { processFileToAssets } from '@/lib/files'
158+
159+
// Strategy 1: Always copy (editor drag-and-drop)
160+
const result = await processFileToAssets({
161+
sourcePath: filePath,
162+
projectPath,
163+
collection,
164+
projectSettings: currentProjectSettings,
165+
copyStrategy: 'always', // Always copies to assets directory
166+
})
167+
168+
// Strategy 2: Only if outside project (ImageField)
169+
const result = await processFileToAssets({
170+
sourcePath: filePath,
171+
projectPath,
172+
collection,
173+
projectSettings: currentProjectSettings,
174+
copyStrategy: 'only-if-outside-project', // Conditionally copies
175+
})
176+
177+
// Returns normalized path and metadata
178+
console.log(result.relativePath) // '/assets/collection/image.png'
179+
console.log(result.wasCopied) // true/false
180+
console.log(result.filename) // 'image.png'
181+
```
182+
183+
**Strategy Pattern Benefits**:
184+
- **Single source of truth**: Eliminates code duplication (removed 125+ duplicate lines)
185+
- **Configurable behavior**: Same function serves different use cases
186+
- **Separation of concerns**: Business logic isolated from UI logic
187+
- **Testable**: Pure functions with comprehensive test coverage
188+
189+
**When to use each strategy**:
190+
- `'always'`: When user explicitly adds files (drag-and-drop, paste)
191+
- `'only-if-outside-project'`: When files might already be in project (manual path edits, existing references)
192+
193+
**UI-specific logic stays in components**:
194+
- Markdown formatting (`formatAsMarkdown`)
195+
- Toast notifications
196+
- React state management
197+
139198
## Critical Patterns
140199

141200
### The `getState()` Pattern (CRITICAL)

‎docs/tasks-todo/task-1-refactor-file-copying-logic.md‎

Lines changed: 82 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -847,16 +847,16 @@ pnpm run check:all
847847

848848
After refactoring:
849849

850-
1. ✓ Both editor drag-and-drop and ImageField use shared `processFileToAssets` utility
851-
2. ✓ Existing behavior preserved:
850+
1. ✅ Both editor drag-and-drop and ImageField use shared `processFileToAssets` utility
851+
2. ✅ Existing behavior preserved:
852852
- Editor: Always copies and renames (date-prefixed kebab-case)
853853
- ImageField: Only copies if outside project
854-
3. ✓ All existing tests pass
855-
4. ✓ New tests cover shared utility (>90% coverage)
856-
5. ✓ Constants consolidated in `src/lib/files/constants.ts`
857-
6. ✓ Documentation updated in architecture guide
858-
7. ✓ No Rust backend changes required
859-
8. ✓ Full quality gate passes: `pnpm run check:all`
854+
3. ✅ All existing tests pass
855+
4. ✅ New tests cover shared utility (>90% coverage)
856+
5. ✅ Constants consolidated in `src/lib/files/constants.ts`
857+
6. ⏳ Documentation updated in architecture guide (in progress)
858+
7. ✅ No Rust backend changes required
859+
8. ✅ Full quality gate passes: `pnpm run check:all`
860860

861861
## Risk Mitigation
862862

@@ -885,3 +885,77 @@ After refactoring:
885885
- ✅ Preserve exact existing behavior
886886
- ✅ Extract reusable logic, keep UI concerns separate
887887
- ✅ Run `pnpm run check:all` after each phase
888+
889+
---
890+
891+
## Implementation Summary
892+
893+
**Status:** ✅ Phases 1-3 Complete | ⏳ Phase 4 In Progress
894+
895+
**Completed:** 2025-01-23
896+
897+
### Phase 1: Shared Infrastructure ✅
898+
899+
**Files Created:**
900+
- `src/lib/files/constants.ts` - IMAGE_EXTENSIONS in two formats
901+
- `src/lib/files/types.ts` - TypeScript interfaces for options and results
902+
- `src/lib/files/fileProcessing.ts` - Core `processFileToAssets()` function
903+
- `src/lib/files/index.ts` - Barrel exports
904+
- `src/lib/files/fileProcessing.test.ts` - 17 comprehensive tests
905+
906+
**Key Features:**
907+
- `copyStrategy` option supporting 'always' and 'only-if-outside-project'
908+
- Path normalization with leading slash
909+
- Uses existing `getEffectiveAssetsDirectory` for path resolution
910+
- Throws errors (no silent fallbacks in shared code)
911+
- All 17 tests passing with >90% coverage
912+
913+
### Phase 2: Editor Drag-and-Drop Refactor ✅
914+
915+
**Files Modified:**
916+
- `src/lib/editor/dragdrop/fileProcessing.ts` - Refactored to use shared utility
917+
- `src/lib/editor/urls/detection.ts` - Updated to use shared constants
918+
- `src/lib/editor/dragdrop/fileProcessing.test.ts` - Updated 28 tests
919+
920+
**Changes:**
921+
- Removed duplicated `IMAGE_EXTENSIONS` constant
922+
- Removed ~50 lines of inline file processing logic
923+
- `processDroppedFile()` now uses `processFileToAssets` with `'always'` strategy
924+
- Preserved editor-specific concerns: markdown formatting, error fallback
925+
- All 28 tests passing
926+
927+
### Phase 3: ImageField Refactor ✅
928+
929+
**Files Modified:**
930+
- `src/components/frontmatter/fields/ImageField.tsx` - Refactored to use shared utility
931+
932+
**Changes:**
933+
- Removed duplicated `IMAGE_EXTENSIONS` constant
934+
- Removed ~75 lines of inline file processing logic
935+
- `handleFileSelect()` now uses `processFileToAssets` with `'only-if-outside-project'` strategy
936+
- Preserved component-specific concerns: toast notifications, loading state
937+
- Fixed TypeScript compatibility with readonly array spreading: `[...IMAGE_EXTENSIONS]`
938+
- All tests passing
939+
940+
### Total Impact
941+
942+
**Code Reduction:**
943+
- Removed ~125+ lines of duplicated code
944+
- Created 1 shared module (4 files, ~100 lines)
945+
- Net reduction: ~25+ lines
946+
- Massive improvement in maintainability
947+
948+
**Test Coverage:**
949+
- Added 17 new tests for shared utility
950+
- Updated 28 existing tests
951+
- All 475 frontend tests passing
952+
- All 89 Rust tests passing
953+
- Zero regressions
954+
955+
**Benefits:**
956+
- Single source of truth for file copying logic
957+
- Changes only need to happen in one place
958+
- Configurable behavior via strategy pattern
959+
- Better separation of concerns
960+
- Easier to test and maintain
961+
- Consistent error handling and path normalization

0 commit comments

Comments
 (0)