IMPORVE ONBOARDING STEPS
This commit is contained in:
Urtzi Alfaro
315
REFACTORING_ROADMAP.md
Normal file
315
REFACTORING_ROADMAP.md
Normal file
@@ -0,0 +1,315 @@
|
||||
# UnifiedOnboardingWizard Refactoring Roadmap
|
||||
|
||||
## Quick Executive Summary
|
||||
|
||||
**Current State**: Functional but architecturally problematic
|
||||
**Main Issue**: UploadSalesDataStep is a 74KB mega-component mixing 8+ different concerns
|
||||
**Debt Level**: HIGH - Technical debt is impacting maintainability
|
||||
|
||||
---
|
||||
|
||||
## Critical Issues (Must Fix)
|
||||
|
||||
### 1. UploadSalesDataStep is Too Large (CRITICAL)
|
||||
- **File Size**: 74,627 bytes (73KB)
|
||||
- **Lines**: 1,577 lines
|
||||
- **State Variables**: 23 useState hooks
|
||||
- **Methods**: Handles file upload, validation, classification, inventory management, stock lots, and more
|
||||
- **Impact**: Almost impossible to test, modify, or reuse components
|
||||
|
||||
**What it should do**:
|
||||
- Single responsibility: Upload and validate file
|
||||
|
||||
**What it actually does**:
|
||||
1. File upload UI and drag-drop
|
||||
2. File validation API call
|
||||
3. AI classification API call
|
||||
4. Inventory item form management (CRUD)
|
||||
5. Stock lot management (CRUD)
|
||||
6. Batch ingredient creation
|
||||
7. Sales data import
|
||||
8. Two entirely different UIs (upload phase vs inventory review phase)
|
||||
|
||||
**Refactoring Approach**:
|
||||
```
|
||||
Current:
|
||||
UploadSalesDataStep (1577 lines, 23 state vars)
|
||||
|
||||
Should be:
|
||||
├─ FileUploadPhase (200 lines)
|
||||
│ ├─ FileDropZone
|
||||
│ ├─ FileValidator
|
||||
│ └─ ProgressIndicator
|
||||
│
|
||||
├─ InventoryManagementPhase (500 lines)
|
||||
│ ├─ InventoryItemList
|
||||
│ ├─ InventoryItemForm
|
||||
│ ├─ StockLotManager
|
||||
│ └─ BatchAddModal
|
||||
│
|
||||
├─ InventoryService (classification, validation)
|
||||
│
|
||||
└─ UploadSalesDataStepContainer (orchestrator, 150 lines)
|
||||
├─ Phase state machine
|
||||
├─ Data flow coordination
|
||||
└─ API integration
|
||||
```
|
||||
|
||||
### 2. Tight Coupling Between Onboarding & Setup (CRITICAL)
|
||||
- UnifiedOnboardingWizard imports from setup-wizard/steps/
|
||||
- Changes to setup steps affect onboarding flow
|
||||
- Makes it impossible to modify setup independently
|
||||
|
||||
**Current Problem**:
|
||||
```typescript
|
||||
// UnifiedOnboardingWizard.tsx line 22-28
|
||||
import {
|
||||
SuppliersSetupStep,
|
||||
RecipesSetupStep,
|
||||
QualitySetupStep,
|
||||
TeamSetupStep,
|
||||
ReviewSetupStep,
|
||||
} from '../setup-wizard/steps'; // ← CROSS-DOMAIN IMPORT
|
||||
```
|
||||
|
||||
**Solution**:
|
||||
- Define step interface contract
|
||||
- Move setup steps to configuration
|
||||
- Load steps dynamically
|
||||
- Create step registry pattern
|
||||
|
||||
### 3. Circular State Dependencies (HIGH)
|
||||
- Steps update wizard context via both onUpdate() and onComplete()
|
||||
- Data flows both directions
|
||||
- No clear source of truth
|
||||
|
||||
**Current Flow**:
|
||||
```
|
||||
UploadSalesDataStep.onUpdate() → wizardContext
|
||||
ProductCategorizationStep.onUpdate() → wizardContext
|
||||
InitialStockEntryStep.onUpdate() → wizardContext
|
||||
All steps also call onComplete() with different data
|
||||
```
|
||||
|
||||
**Should Be**:
|
||||
```
|
||||
Step.onUpdate() → Local UI state (transient)
|
||||
Step.onComplete() → Single source of truth (wizardContext + Backend)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## High Priority Issues (Fix in Next Sprint)
|
||||
|
||||
### 4. Duplicate Visibility Logic
|
||||
**Locations**:
|
||||
- UnifiedOnboardingWizard.tsx (line 59-165): Step conditions
|
||||
- WizardContext.tsx (line 183-189): getVisibleSteps() logic
|
||||
- Same rules repeated in two places
|
||||
|
||||
**Fix**: Extract to single configuration file
|
||||
|
||||
### 5. Auto-Completion Logic is Scattered
|
||||
- user_registered auto-completion: OnboardingWizardContent.useEffect() (line 204)
|
||||
- suppliers-setup auto-completion: UnifiedOnboardingWizard.handleStepComplete() (line 349)
|
||||
- No clear policy or pattern
|
||||
|
||||
**Fix**: Centralized auto-completion policy engine
|
||||
|
||||
### 6. State Management Confusion
|
||||
- UI state mixed with data state in context
|
||||
- No clear boundary between component state and context state
|
||||
- Difficult to understand data persistence
|
||||
|
||||
**Fix**:
|
||||
- UI state (isAdding, editingId, showInventoryStep) → Component local state
|
||||
- Data state (aiSuggestions, categorizedProducts) → Context
|
||||
- Session state → Backend API
|
||||
|
||||
---
|
||||
|
||||
## Medium Priority Issues (Fix Over Time)
|
||||
|
||||
### 7. No Inter-Step Communication Contract
|
||||
**Problem**: Different components use different interfaces for same data
|
||||
- InventoryItemForm (line 28-50)
|
||||
- Product (ProductCategorizationStep line 7-14)
|
||||
- ProductWithStock (InitialStockEntryStep line 8-15)
|
||||
- Ingredient (API type)
|
||||
- Stock (API type)
|
||||
|
||||
All represent similar data but with different shapes.
|
||||
|
||||
**Fix**: Create unified type system for inventory throughout flow
|
||||
|
||||
### 8. Complex Data Transformations Without Clear Schema
|
||||
Data travels through pipeline with implicit transformations:
|
||||
```
|
||||
File → ImportValidationResponse → ProductSuggestionResponse
|
||||
→ InventoryItemForm → Ingredient → Stock → categorizedProducts
|
||||
→ Product (different type!) → ProductWithStock
|
||||
```
|
||||
|
||||
**Fix**: Create explicit transformation functions between steps
|
||||
|
||||
---
|
||||
|
||||
## Files to Delete (Immediate)
|
||||
|
||||
1. **OnboardingWizard.tsx** (567 lines, 19.5KB)
|
||||
- Status: Deprecated, not used
|
||||
- Usage: Only in exports, never imported
|
||||
- Risk: LOW - completely safe to delete
|
||||
- Action: DELETE and remove from index.ts exports
|
||||
|
||||
2. **SetupPage.tsx** (18 lines, 567 bytes) - OPTIONAL
|
||||
- Status: Deprecated by design
|
||||
- Usage: Route /app/setup still references it
|
||||
- Risk: MEDIUM - needs migration plan
|
||||
- Action: Either delete or redirect to /app/onboarding
|
||||
|
||||
3. **Setup route in routes.config.ts** (line 578-593) - OPTIONAL
|
||||
- Status: Redundant with /app/onboarding
|
||||
- Action: Remove or convert to redirect
|
||||
|
||||
---
|
||||
|
||||
## Refactoring Priority & Timeline
|
||||
|
||||
### Phase 1: Quick Wins (Week 1)
|
||||
**Time**: 2-3 hours
|
||||
**Impact**: LOW immediate, HIGH long-term
|
||||
|
||||
- [ ] Delete OnboardingWizard.tsx
|
||||
- [ ] Remove from index.ts exports
|
||||
- [ ] Extract visibility rules to config file
|
||||
- [ ] Add comments documenting current state
|
||||
|
||||
### Phase 2: State Management Cleanup (Week 2)
|
||||
**Time**: 3-4 hours
|
||||
**Impact**: MEDIUM
|
||||
|
||||
- [ ] Document which state belongs where (UI vs Context vs Backend)
|
||||
- [ ] Move UI state out of context (isAdding, editingId, etc.)
|
||||
- [ ] Clear data flow direction (parent → child → parent only)
|
||||
|
||||
### Phase 3: Split UploadSalesDataStep (Sprint)
|
||||
**Time**: 6-8 hours
|
||||
**Impact**: CRITICAL
|
||||
|
||||
- [ ] Extract FileUploadPhase component
|
||||
- [ ] Extract InventoryManagementPhase component
|
||||
- [ ] Extract classification logic to service
|
||||
- [ ] Create container component to orchestrate
|
||||
- [ ] Add unit tests for each extracted component
|
||||
|
||||
### Phase 4: Decouple Onboarding from Setup (Sprint)
|
||||
**Time**: 4-6 hours
|
||||
**Impact**: HIGH
|
||||
|
||||
- [ ] Define step interface contract
|
||||
- [ ] Create step registry/configuration
|
||||
- [ ] Move setup steps to configuration
|
||||
- [ ] Load steps dynamically
|
||||
|
||||
### Phase 5: Inter-Step Communication (Sprint)
|
||||
**Time**: 4-6 hours
|
||||
**Impact**: MEDIUM
|
||||
|
||||
- [ ] Define unified inventory type system
|
||||
- [ ] Create transformation functions between steps
|
||||
- [ ] Add type validation at step boundaries
|
||||
- [ ] Document data contracts
|
||||
|
||||
---
|
||||
|
||||
## Current Architecture Summary
|
||||
|
||||
### Strengths
|
||||
1. **Main Wizard Orchestrator is Well-Designed**
|
||||
- Clean step navigation
|
||||
- Good progress tracking
|
||||
- Proper initialization logic
|
||||
- Clear completion flow
|
||||
|
||||
2. **WizardContext is Reasonable**
|
||||
- Good use of React Context API
|
||||
- Most step data stored centrally
|
||||
- Safe from localStorage corruption
|
||||
|
||||
3. **Most Individual Steps are Well-Designed**
|
||||
- ProductCategorizationStep: Good UI/UX
|
||||
- InitialStockEntryStep: Simple and effective
|
||||
- BakeryTypeSelectionStep: Beautiful design
|
||||
|
||||
### Weaknesses
|
||||
1. **UploadSalesDataStep is a Monster**
|
||||
- Mixes file upload + inventory management
|
||||
- Too many state variables
|
||||
- Difficult to test
|
||||
- Difficult to reuse
|
||||
|
||||
2. **Module Boundaries are Blurred**
|
||||
- Onboarding imports from setup-wizard
|
||||
- Difficult to modify either independently
|
||||
- Violates separation of concerns
|
||||
|
||||
3. **State Management is Confusing**
|
||||
- UI state mixed with data state
|
||||
- Bidirectional data flow
|
||||
- Multiple sources of truth
|
||||
|
||||
4. **No Clear Contracts Between Steps**
|
||||
- Data types change between steps
|
||||
- Implicit transformations
|
||||
- Runtime type errors possible
|
||||
|
||||
---
|
||||
|
||||
## How to Use This Document
|
||||
|
||||
1. **For Planning**: Use Phase 1-5 to plan sprints
|
||||
2. **For Understanding**: Read ARCHITECTURE_ANALYSIS.md for detailed breakdown
|
||||
3. **For Implementation**: Follow the refactoring approach in each section
|
||||
4. **For Maintenance**: Reference this after refactoring to keep architecture clean
|
||||
|
||||
---
|
||||
|
||||
## Success Criteria
|
||||
|
||||
After refactoring, the codebase should have:
|
||||
|
||||
- [ ] UploadSalesDataStep is <300 lines
|
||||
- [ ] No imports between onboarding and setup-wizard domains
|
||||
- [ ] Clear separation of UI state and data state
|
||||
- [ ] All steps follow same interface contract
|
||||
- [ ] Data transformations are explicit and testable
|
||||
- [ ] Unit test coverage >80% for all step components
|
||||
- [ ] All 8 architectural problems resolved
|
||||
- [ ] Deprecated files deleted (OnboardingWizard.tsx)
|
||||
|
||||
---
|
||||
|
||||
## Estimated Effort
|
||||
|
||||
| Phase | Effort | Impact | Timeline |
|
||||
|-------|--------|--------|----------|
|
||||
| Phase 1 (Quick Wins) | 2-3 hours | LOW | Week 1 |
|
||||
| Phase 2 (State Management) | 3-4 hours | MEDIUM | Week 2 |
|
||||
| Phase 3 (Split Component) | 6-8 hours | CRITICAL | Sprint 2 |
|
||||
| Phase 4 (Decouple Domains) | 4-6 hours | HIGH | Sprint 3 |
|
||||
| Phase 5 (Data Contracts) | 4-6 hours | MEDIUM | Sprint 4 |
|
||||
| **TOTAL** | **19-27 hours** | **HIGH** | **4-5 Weeks** |
|
||||
|
||||
---
|
||||
|
||||
## Contact & Questions
|
||||
|
||||
For detailed architecture information, see: `ARCHITECTURE_ANALYSIS.md`
|
||||
|
||||
Key sections:
|
||||
- Section 2: Detailed architectural issues with code examples
|
||||
- Section 3: Step dependencies and data flow diagrams
|
||||
- Section 6: Problem statements for each issue
|
||||
- Section 8: Detailed recommendations for each refactoring task
|
||||
|
||||
Reference in New Issue
Block a user