Eric Taylor
fbdf450c14
## Code Quality Improvements ### Global State Removal - Eliminated global drag_info dictionary - Moved drag_state management into TimeTracker class - Removed all global drag_info dependencies ### Updated Components - **ClickableCell constructor**: Added time_tracker parameter for proper reference - **ClickableCell methods**: Updated to use self.time_tracker.drag_info - **TimeTracker methods**: Updated on_global_drag() and on_global_up() - **Instance creation**: Updated ClickableCell instantiation calls ### Benefits Achieved - **Better Encapsulation**: State properly contained within class boundaries - **Thread Safety**: Reduced race conditions from shared global state - **Testability**: Individual instance testing now possible - **Instance Isolation**: Multiple TimeTracker instances work independently - **Maintainability**: Clearer code structure with explicit dependencies ### Verification - ✅ All drag functionality preserved (paint/erase operations) - ✅ Drag state management works correctly - ✅ Multiple instances properly isolated - ✅ All 6 existing test suites pass (no regressions) - ✅ New comprehensive test suite created and passing - ✅ Application starts and runs correctly ## Files Modified - **time_tracker.py**: Global state removal and class attribute implementation - **AGENTS.md**: Updated coding guidelines for class preferences - **TODO.md**: Marked drag_info task as completed, updated progress - **tests/test_drag_info_class_attribute.py**: New comprehensive test suite ## Testing - Added complete test suite for drag_info functionality - Tests verify global state removal and class attribute access - Confirms multiple instance isolation - Validates drag state management Code quality significantly improved with zero functional regressions.
218 lines
8.7 KiB
Markdown
218 lines
8.7 KiB
Markdown
# Time Tracker Refactor TODO
|
|
|
|
This document outlines the recommended improvements from the code review, organized by priority stages.
|
|
|
|
## ✨ New Feature Ideas
|
|
|
|
### User Interface Enhancements
|
|
- [x] **Alternating colors for each hour** ✅
|
|
- Improve visual distinction between time slots
|
|
- Use subtle color gradients or patterns
|
|
- Consider color-blind friendly palettes
|
|
|
|
- [ ] **Pinned/frozen columns for data entry** 🚧
|
|
- Freeze Job, Task Name, Notes, Customer columns on the left side
|
|
- These columns remain visible when horizontally scrolling through time slots
|
|
- Similar to spreadsheet frozen panes functionality
|
|
- Critical for usability with work schedules longer than 4-5 hours
|
|
|
|
- [ ] **More compact user interface**
|
|
- Reduce padding and margins for better space utilization
|
|
- Collapsible sections for advanced features
|
|
- Responsive layout for smaller screens
|
|
|
|
- [ ] **Button to open archive CSV in text editor**
|
|
- Add "Open Archive" button that launches system default editor
|
|
- Cross-platform support (Windows, Mac, Linux)
|
|
- Optional: Use vim/emacs if available
|
|
|
|
- [ ] **Windows compatibility improvements**
|
|
- Test and fix Windows-specific path handling
|
|
- Ensure proper font rendering
|
|
- Verify installer/packaging options
|
|
|
|
## 🚨 Stage 1: Critical Issues (High Priority)
|
|
|
|
### Security & Stability
|
|
- [x] **Add comprehensive input sanitization** ✅
|
|
- Validate task names, notes, customer names
|
|
- Prevent CSV injection attacks
|
|
- Sanitize file paths
|
|
- **Implementation**: Created sanitization functions for CSV text, filenames, and config data
|
|
- **Security Features**: Excel formula blocking, directory traversal protection, JSON safety
|
|
|
|
- [x] **Critical security fixes from code review** ✅
|
|
- [x] **Fix settings file race condition** - Use atomic write pattern with temp file ✅
|
|
- **Issue**: Direct file overwrite can corrupt settings if process crashes
|
|
- **Impact**: Loss of all application configuration (jobs, customers, paths)
|
|
- **Solution**: Atomic write with temp file + os.replace() + cleanup
|
|
- [x] **Add CSV quoting protection** - Use proper csv.QUOTE_MINIMAL for safer CSV writing ✅
|
|
- **Issue**: Current character removal isn't enough for complete CSV safety
|
|
- **Impact**: Potential CSV injection attacks could still succeed
|
|
- **Solution**: Applied csv.QUOTE_MINIMAL to both CSV DictWriter operations
|
|
- [x] **Sanitize all CSV fields consistently** - Fix Date field and username field gaps ✅
|
|
- **Issue**: Some fields (Date, username) not properly sanitized before CSV writing
|
|
- **Impact**: Data corruption and potential inject vulnerabilities remain
|
|
- **Solution**: Created sanitize_date_text() and applied sanitization to all fields
|
|
|
|
- [ ] **Replace filedialog usage** for PDF exports
|
|
- Use `filedialog.asksaveasfilename` instead
|
|
- Validate file extensions
|
|
- Add overwrite confirmation
|
|
|
|
- [x] **Move drag_info from global to class attribute** ✅
|
|
- Remove global state dependency
|
|
- Improve encapsulation
|
|
- Make class more testable
|
|
|
|
- [x] **Move drag_info from global to class attribute in TimeTracker** ✅
|
|
|
|
🔧 Medium Priority Additions (from code review):
|
|
- [ ] **Add type conversion error handling** - Prevent ValueError on hours field
|
|
- [ ] **Precompile regular expressions** for better performance
|
|
- [ ] **Add comprehensive error handling** for filename and filesystem issues
|
|
|
|
### Code Structure
|
|
- [ ] **Refactor open_settings() method** (200+ lines)
|
|
- Extract tab creation into separate methods: `_create_jobs_tab()`, `_create_customers_tab()`, etc.
|
|
- Extract button creation logic
|
|
- Reduce complexity
|
|
|
|
- [ ] **Refactor export_to_pdf() method**
|
|
- Extract table creation logic
|
|
- Extract styling logic
|
|
- Simplify main method flow
|
|
|
|
## 🔧 Stage 2: Architecture Improvements (Medium Priority)
|
|
|
|
### Performance & UX
|
|
- [ ] **Implement pinned/frozen columns for data entry** (NEW)
|
|
- **Problem**: When users have long work schedules (8+ hours), they can't see the Job/Task/Notes/Customer columns while scrolling through later time slots
|
|
- **Solution**: Create dual-frame layout with fixed left pane for data columns and scrollable right pane for time slots
|
|
- **Implementation**:
|
|
- Split scrollable_frame into two frames: `fixed_columns_frame` (columns 0-3) and `time_columns_frame` (column 4+)
|
|
- `fixed_columns_frame`: No horizontal scroll, contains Job dropdown, Task entry, Notes entry, Customer dropdown
|
|
- `time_columns_frame`: Horizontal scroll for time slots, aligning with fixed columns vertically
|
|
- Synchronize vertical scrolling between both frames
|
|
- **Technical challenges**:
|
|
- Row height synchronization between frames
|
|
- Visual alignment and border management
|
|
- Drag operations spanned across both frames
|
|
- Focus management and tab ordering
|
|
- [ ] **Implement CSV streaming reader** for large archive files
|
|
- Prevent memory issues with large datasets
|
|
- Add pagination for large archives
|
|
- Consider SQLite for better performance
|
|
|
|
- [ ] **Add progress dialogs** for long-running operations
|
|
- PDF export progress
|
|
- Large CSV processing
|
|
- Archive operations
|
|
|
|
- [ ] **Create Settings class** for type-safe configuration
|
|
- Replace JSON dict manipulation
|
|
- Add validation for settings values
|
|
- Provide default value management
|
|
|
|
### Code Organization
|
|
- [ ] **Extract constants** to separate module
|
|
- Widget dimensions, colors, time intervals
|
|
- File paths and formats
|
|
- Magic numbers scattered in code
|
|
|
|
- [ ] **Create GUIBuilder helper class** for common widget operations
|
|
- Standardize widget creation
|
|
- Reduce code duplication
|
|
- Consistent styling
|
|
|
|
- [ ] **Create ReportGenerator class**
|
|
- Extract reporting logic from TimeTracker
|
|
- Separate data processing from GUI
|
|
- Make reports more testable
|
|
|
|
- [ ] **Add data validation layer** between GUI and CSV operations
|
|
- Centralized validation logic
|
|
- Consistent error messages
|
|
- Better separation of concerns
|
|
|
|
### Testing
|
|
- [ ] **Add comprehensive unit tests** for data processing methods
|
|
- CSV reading/writing
|
|
- Time calculation logic
|
|
- Data validation
|
|
- Report generation
|
|
|
|
## 🎯 Stage 3: Quality & Nice-to-Haves (Low/Medium Priority)
|
|
|
|
### Code Quality
|
|
- [ ] **Add type hints** throughout the codebase
|
|
- Improve IDE support
|
|
- Better documentation
|
|
- Catch type-related bugs early
|
|
|
|
- [ ] **Implement debouncing** for rapid cell selections during drag operations
|
|
- Improve performance during fast dragging
|
|
- Reduce GUI update frequency
|
|
- Better user experience
|
|
|
|
### Documentation & Maintenance
|
|
- [ ] **Create architecture documentation**
|
|
- Document class relationships
|
|
- Add sequence diagrams for key workflows
|
|
- Maintenance guidelines
|
|
|
|
## 📋 Implementation Guidelines
|
|
|
|
### Before Starting Each Task:
|
|
1. Create a feature branch for the task
|
|
2. Run existing tests to ensure baseline
|
|
3. Implement changes incrementally
|
|
4. Test each change thoroughly
|
|
5. Update AGENTS.md if adding new patterns
|
|
|
|
### Pinned Columns Implementation Strategy:
|
|
For the pinned columns feature specifically:
|
|
1. **Phase 1**: Create dual-frame layout structure
|
|
- Replace single `scrollable_frame` with linked `fixed_columns_frame` and `time_columns_frame`
|
|
- Ensure proper vertical alignment between frames
|
|
2. **Phase 2**: Update row creation logic
|
|
- Modify `add_row()` to create widgets in both frames
|
|
- Maintain row index synchronization
|
|
3. **Phase 3**: Synchronize interactions
|
|
- Update drag operations to work across frame boundaries
|
|
- Ensure consistent styling and borders
|
|
- Test focus management and keyboard navigation
|
|
|
|
### After Each Task:
|
|
1. Run all tests to ensure no regressions
|
|
2. Test the GUI functionality manually
|
|
3. Run the application to verify it works end-to-end
|
|
4. Update this TODO file with completion status
|
|
|
|
### Testing Strategy:
|
|
- **Unit Tests**: For individual methods and classes
|
|
- **Integration Tests**: For CSV operations and report generation
|
|
- **GUI Tests**: Manual testing of user workflows
|
|
- **Regression Tests**: Ensure existing functionality isn't broken
|
|
|
|
## 🔄 Dependencies Between Tasks:
|
|
|
|
| Task | Depends On |
|
|
|------|------------|
|
|
| Create ReportGenerator | Add data validation layer |
|
|
| Add data validation layer | Create Settings class |
|
|
| Implement CSV streaming | Create constants module |
|
|
| **Pinned columns** | **Create GUIBuilder helper class** (for consistent widget management) |
|
|
| **Add critical security fixes** | **None (time-sensitive)** |
|
|
| **Add medium priority fixes** | **None (performance/stability)** |
|
|
| Add comprehensive tests | All major refactoring tasks |
|
|
|
|
## 📊 Progress Tracking:
|
|
|
|
- **Stage 1**: 6/8 completed (1 base + 5 critical fixes completed)
|
|
- **Stage 2**: 0/9 completed
|
|
- **Stage 3**: 1/2 completed
|
|
- **New Features**: 1/4 completed
|
|
- **Total**: 7/24 completed
|
|
|
|
*Priority Legend: 🚨 Critical | 🔧 Important | 🎯 Enhancement | 🔧 🔧 Code Review Findings* |